Opened 15 years ago
Closed 15 years ago
#11588 closed enhancement (duplicate)
show_message() should flush the output buffers when its called.
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.9 |
Component: | Administration | Keywords: | |
Focuses: | Cc: |
Description
show_message() should flush the output buffers when its called, If a function is calling show_message(), its safe to assume that the function is wanting the output displayed now.
show_message() is mostly used within the various Upgraders.
See patch.
Attachments (1)
Change History (20)
#2
@
15 years ago
another option is wp_ob_end_flush_all()
.
Which has the benefit of disabling any php-level ob_*()'s, the main(and only) benefit is that if a gzip ob_* filter is used, then the client will get the message ASAP.. as the gzip gets disabled.
That is also its downside however.. removes the ability to ob_start(); ob_get_contents(); the entire upgrade process...
#3
@
15 years ago
FWIW, the suggested patch doesn't flush hard enough.
I use this function, personally:
function force_flush() { echo "\n\n<!-- Deal with browser-related buffering by sending some incompressible strings -->\n\n"; for ( $i = 0; $i < 5; $i++ ) echo "<!-- abcdefghijklmnopqrstuvwxyz1234567890aabbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz11223344556677889900abacbcbdcdcededfefegfgfhghgihihjijikjkjlklkmlmlnmnmononpopoqpqprqrqsrsrtstsubcbcdcdedefefgfabcadefbghicjkldmnoepqrfstugvwxhyz1i234j567k890laabmbccnddeoeffpgghqhiirjjksklltmmnunoovppqwqrrxsstytuuzvvw0wxx1yyz2z113223434455666777889890091abc2def3ghi4jkl5mno6pqr7stu8vwx9yz11aab2bcc3dd4ee5ff6gg7hh8ii9j0jk1kl2lmm3nnoo4p5pq6qrr7ss8tt9uuvv0wwx1x2yyzz13aba4cbcb5dcdc6dedfef8egf9gfh0ghg1ihi2hji3jik4jkj5lkl6kml7mln8mnm9ono -->\n\n"; while ( ob_get_level() ) ob_end_flush(); @ob_flush(); @flush(); @set_time_limit(FS_TIMEOUT); } # force_flush()
and before it, at the very beginning of the various upgrader scripts:
if ( function_exists('apache_setenv') ) @apache_setenv('no-gzip', 1); @ini_set('zlib.output_compression', 0); @ini_set('implicit_flush', 1);
anything short of that will reveal that the flushing does not occur on some servers and some user agents.
#4
follow-up:
↓ 5
@
15 years ago
I agree that its not enough to completely flush it on every system.
I would however, Like to suggest that we agree on a very basic start which works on the majority of servers without switching to changing the environment or working around various browser bugs.
Once the basic issue has been dealt with, Then we can consider the extra's to make it work accross other specific configurations.
Personally, I'd like to suggest this from your quote:
@ob_flush(); @flush();
#5
in reply to:
↑ 4
@
15 years ago
Replying to dd32:
I agree that its not enough to completely flush it on every system.
I would however, Like to suggest that we agree on a very basic start which works on the majority of servers without switching to changing the environment or working around various browser bugs.
Once the basic issue has been dealt with, Then we can consider the extra's to make it work accross other specific configurations.
Up to you. I hook into show_message() using the above-mentioned function anyway. :-P
#6
follow-up:
↓ 7
@
15 years ago
Up to you. I hook into show_message() using the above-mentioned function anyway. :-P
I realise that, But this is a defect for a bug in an open source application that thousands use, Your input is valued, But please do think of regular users rather than just those of your distribution if you're going to offer knowledge.
If you have a better understanding of the situation than I, as you obviously do, Then i'd prefer you to offer a solution. But i'd like to see a minimalist solution which fixes it under the most basic servers first, and moves on from there.
#7
in reply to:
↑ 6
@
15 years ago
Replying to dd32:
Up to you. I hook into show_message() using the above-mentioned function anyway. :-P
I realise that, But this is a defect for a bug in an open source application that thousands use, Your input is valued, But please do think of regular users rather than just those of your distribution if you're going to offer knowledge.
I thought no less, and that was precisely my point in suggesting the above code. I originally tried to flush things more or less the way you are suggesting. It only works provided that:
- The server does not enable output buffering
- The server does not gzip the output (meaning it technically ignores flush() statements)
- The user agent doesn't wait for a certain amount of data before rendering (which is to say most UA nowadays)
The code I'm suggesting has been evolving since towards early 2007. It has been tested by hundreds of users.
Again, it's just for information. You're welcome to use any or all of it; or to ignore it, and delay your arriving to similar code in a few months or years.
#8
@
15 years ago
Just for the note, too: the line in there that increases the timeout has more to do with preventing the upgrade from failing due to server timeouts that it has to do with flushing. The full code that I use actually reconnects the FTP whenever a message is displayed, in order to work around the fact that FTP times out on a per session basis. I'll post that code too if there is any interest.
#9
@
15 years ago
- Keywords dev-feedback added
You're welcome to use any or all of it; or to ignore it, and delay your arriving to similar code in a few months or years.
I see it as overkill to go around the mulberry bush so many times, We did this exact same thing last time on this very issue 14 months ago. End result was no commiter has been willing to go near the ticket.
I do not believe that it needs to be flushed under every single different environment, If a user has enabled output compression or a server has server-level ziping in place, There is not reliable way to prevent that, Those in your code will prevent it some of the time. thats it.
Can a core-dev step in here and take what they think from this lot and commit something? Anything? If it gets fixed for 80% (Or heck, even 50%) its a good start to work off.
#10
@
15 years ago
I suggest to close this ticket as invalid. Output buffering is no-where used inside the core code and we should not introduce it in a function only because that function is doing output. I mean we have tons of output related functions. do we flush there?
What are the criterions? Output buffering should be completely ignored to have at least a save spot for hackers.
#11
@
15 years ago
- Type changed from defect (bug) to feature request
It's more of a feature request than a bug.
#12
@
15 years ago
show_message() is specifically for the upgraders. Upgrading can take a while, and we deliberately call show_message() to write information to the browser updating the user on the status of the upgrades, as the package is downloaded, unpacked, etc.
From what I have gathered from dd32, the original intention of show_message() was to send output to the browser. Not including a flush() here was an accidental omission when the upgraders were written.
Hence why we should flush here even though we don't anywhere else in core.
#13
follow-up:
↓ 14
@
15 years ago
This is a solution, but to the wrong problem. The correct approach would be to make the upgrade progress called async through JS with (hopefully) frequent status updates.
#14
in reply to:
↑ 13
@
15 years ago
Replying to matt:
This is a solution, but to the wrong problem. The correct approach would be to make the upgrade progress called async through JS with (hopefully) frequent status updates.
actually, async isn't the best of idea for mass plugin upgrades. you can end up in a situation where concurrent loads update the active_plugins option and mess it up. those need to be handled sequentially.
what could be reasonable to do is to handle the upgrade process in its own iframe. but that ought to be a separate ticket.
i'm not sure if mearly
flush()
is the best function to call.. checking previous tickets on this..