Attachment 87307 [details] [diff] (async reflow and painting for text widgets), which landed as part of bug 141900, was temporarily disabled with attachment 101640 [details] [diff] [review] during mozilla1.2beta because of some problems it exposed, which fell into 3 general categories: 1. Typed text lags in text widgets, especially in the URL bar. (Bug 158782) 2. Caret flashes at start of line when deleting. (Bug 151882) 3. Updating textfields via JS causes flashing. (Bug 165130) Though bugs 158782, 151882, and 165130 are currently marked as fixed, they will have to be addressed properly before backing out attachment 101640 [details] [diff] [review] to re-enable async reflows and paints, so they will be listed as blockers for this bug. Note that item #1 is not much of a problem on Win32 platforms because of the fix for bug 163528 which forces syncronous reflows and paints at the widget level for better dhtml performance.

Here's an excerpt from an email I sent to drivers@mozilla.org that explains why some of these issues were happening ... Issues #1 and #3 seem to be related to the speed of the processor and the priority in which PLEvent and OS events are processed on each platform. Issue #1 is also more visible in auto-complete textfields such as the URL bar, which I'm guessing is due in part to the lookup timer it uses. kmcclusk@netscape.com has done some work to adjust the PLEvent/OSEvent priorities on Win32 to make the app more responsive and prevent paint starvation to improve dhtml performance. Some of his fixes, like the one for bug 163528, hide the lag on Win32 platforms because there is now some code at the widget level that forces syncronous reflows and paints while processing certain events. As far as I'm aware, there is no one signed up yet to do this same kind of work on Linux or Mac, which is where the majority of the complaints are coming from. It should be noted that though the text doesn't appear immediately as you type, it appears as soon as you stop typing or slow down enough so that the paint events can be processed between keystrokes, no matter how much text is present in the widget. This seems to be the thing that disturbs people the most. With this patch turned off, there will be situations where you can type ahead of what is displayed on screen (once again more apparent in the URL bar textfield), but the difference will be that when you stop, you'll have to wait till the textfield catches up, reflowing and painting one character at a time. Issue #2 is due to the fact that caret show/hide code is synchronous, and to complicate things even further, there are several places in layout and editor that hide/show the caret during a single edit operation up to 3-4 times. I started to work on cleaning some of this up as part of bug 151882, which does have a patch, but the issue of sync'ing up the caret with async reflows and paints still has to be addressed. I've got some ideas on how to fix the async issue, but it's nothing I want to try to implement/land for 1.2. All this said, I've posted a one line patch (attachment 101640 [details] [diff] [review]) in bug 141900, which simply avoids setting a bit on the editor, which disables attachment 87307 [details] [diff] [review] completely. With this patch text widgets will still be slightly faster than those used in the Mozilla 1.0 release, due to some of the other patches that landed as part of bug 141900, but slightly slower than those in Mozilla 1.1 as mentioned above.

Some of the issues this bug ran into could be fixed by bug 244366.

So I just tried doing this.... Now that bug 244366 is fixed, I had no issues with bug 165130. I really don't type fast enough to tell about bug 158782. And I couln't reproduce bug 151882 as described, but I _did_ see a tad of caret flicker while quickly typing random stuff in a textarea. The caret flickered to somewhere about in the middle of the next-to-last line. Blake, roc, any ideas? I'd really like to make this change if we can, and we're _so_ close. Note that editor does use NS_VMREFRESH_DEFERRED when reenabling updates. I could make it do NO_SYNC and that might help, but would lose the point, since it would cause reflows to flush out at every character...

we'd really like this. roc, kbap can you tell us how close this is?

Caret flicker may have improved with mrbkap's big caret fix. I haven't tried this myself so I can't say anything firsthand. Can you recruit someone to test by backing out attachment 101640 [details] [diff] [review] and seeing how it goes?

> Caret flicker may have improved with mrbkap's big caret fix. It didn't. I tested back when that landed.

Blake, can you please dig into this?

So, I can't seem to reproduce any sort of caret flicker on Linux. I'll try OS X when I update my build.

This worksforme on Linux too, with current trunk. Let's give it a shot?

Fix checked into trunk.

I had to back this out -- it was causing reftests to fail.

Which ones?

That's mochitest... but in any case. That looks like focusing the form control didn't put the caret after all the content, and the rest of the failures follow. I guess caret placement depends on the frame tree, right? Perhaps we should flush reflow before trying to move the caret...

Yeah, sorry. I was busy trying to deal with other orange as well. I figured that we probably need to flush layout somewhere -- I'll investigate tomorrow.

Blake, when is that 'tomorrow' ;)

Doesn't look like we are getting anywhere and this pre-dates 1.9. Moving to the wanted list - re-nom if you disagree..

This should block because it blocks bug 586662. mrbkap, I trust that you're not still working on this...

I ran the entire test suite locally on Windows with this patch, and it seems pretty safe. I also couldn't see any manifestation of bug 158782, bug 151882 or bug 165130 with this patch. In addition, this indeed seems to fix the problem in bug 586662, so I think we really want to take this.