Closed Bug 457862 Opened 11 years ago Closed 11 years ago
onresize events should fire at every resize while resizing the window, not just at 200ms intervals
416 bytes, text/html
65.57 KB, text/html
5.25 KB, patch
|Details | Diff | Splinter Review|
347 bytes, text/html
12.59 KB, patch
|Details | Diff | Splinter Review|
Build ID: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080930050845 Minefield/3.1b1pre Bug 114649 fixed this issue partially, but it still does not trigger as often as IE and WebKit based browsers. See bug 114649, comment 28 for a possible planned future solution.
So it sounds like this depends on Bug 374980.
Summary: onresize events should fire more often while resizing the window → onresize events should fire at every resize while resizing the window, not just at 200ms intervals
Competition is good. I'm guessing this won't fit in 1.9.1, but it would be good to hear of any short-term mitigation options. /be
This is not yet fully tested, but behavior and speed should be ok. Fixes 'Visual Testcase', actually it behaves better than on webkit where resize event isn't apparently fired often enough in some cases. Based on that test Opera does *not* fire resize continuously. I don't understand why firing resize event should depend on compositor.
Especially I haven't tested how attachment 355212 [details] [diff] [review] works if page contains frames.
My comment #3 was based on this: https://bugzilla.mozilla.org/show_bug.cgi?id=114649#c19
I think the rationale for depending on compositor is that we might be doing layout too often to begin with, whereas doing it at reasonably-spaced intervals might help keep the cost down of running script as well. That said, we're probably ok just doing this.
Smaug: thanks! Let's get this into 1.9.1 if at all possible. /be
I made just as small changes as possible to the testcase, which expects async resize.
Does test_hover.html really need setTimeout(..., 100) rather that setTimeout(..., 0) ?
with 100 it was more reliable if I accidentally moved mouse a bit.
I'd have expected the opposite. Were you moving the mouse within the test document both times? Either way, the test isn't going to work if you're moving the mouse, I don't think, so I'd prefer keeping it faster if possible.
And with test_hover.html, could you make sure that the test still fails if you comment out the FlushPendingNotifications call in PresShell::DispatchSynthMouseMove ?
The test fails if PresShell::DispatchSynthMouseMove is commented out.
Comment on attachment 355285 [details] [diff] [review] even simpler + needed changes to test_hover.html OK, r+sr=dbaron, although I think I'd prefer the timeout in the test to be 0 rather than 100.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Is there a followup bug for setTimeout 100 vs 0 not making the expected difference, or not being desirable in the test, or something? /be
This fixed Windows bug 460620 (one more arument to have it on branch)
This should go into 1.9.1 -- IMHO! /be
What about just doing an async event instead of using a scriptrunner? So just post the runnable into the event queue?
Yeah, need to try that.
Unfortunately that doesn't work.
What's the performance impact when resizing a window here? Measurable?
The regression is fixed, right? Any reason not to take this on 1.9.1?
Maybe bug 473805?
smaug, do you think we should wait on bug 473805 to land this on 1.9.1?
Comment on attachment 355285 [details] [diff] [review] even simpler + needed changes to test_hover.html Clearing nom per Smaug's comment. Please renominate once bug 473805 has landed.
Whiteboard: [parity-ie] [parity-chrome] [parity-safari] → [parity-ie] [parity-chrome] [parity-safari][needs 191 approval after bug 473805]
Whiteboard: [parity-ie] [parity-chrome] [parity-safari][needs 191 approval after bug 473805] → [parity-ie] [parity-chrome] [parity-safari]
Whiteboard: [parity-ie] [parity-chrome] [parity-safari] → [parity-ie] [parity-webkit]
You need to log in before you can comment on or make changes to this bug.