Closed
Bug 457862
Opened 16 years ago
Closed 16 years ago
onresize events should fire at every resize while resizing the window, not just at 200ms intervals
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.1 | --- | wontfix |
People
(Reporter: bugzilla, Assigned: smaug)
References
Details
(Whiteboard: [parity-ie] [parity-webkit])
Attachments
(5 files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
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.
Reporter | ||
Comment 1•16 years ago
|
||
Reporter | ||
Updated•16 years ago
|
Whiteboard: [parity-ie] [parity-chrome] [parity-safari]
Comment 3•16 years ago
|
||
So it sounds like this depends on Bug 374980.
Reporter | ||
Updated•16 years ago
|
Depends on: compositor
Updated•16 years ago
|
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
Comment 5•16 years ago
|
||
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
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Assignee | ||
Comment 6•16 years ago
|
||
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.
Assignee | ||
Comment 7•16 years ago
|
||
Especially I haven't tested how attachment 355212 [details] [diff] [review] works if page contains frames.
Comment 8•16 years ago
|
||
My comment #3 was based on this:
https://bugzilla.mozilla.org/show_bug.cgi?id=114649#c19
Comment 9•16 years ago
|
||
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.
Comment 10•16 years ago
|
||
Smaug: thanks!
Let's get this into 1.9.1 if at all possible.
/be
Flags: wanted1.9.1?
Assignee | ||
Comment 11•16 years ago
|
||
Assignee | ||
Comment 12•16 years ago
|
||
I made just as small changes as possible to the testcase, which expects
async resize.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #355285 -
Flags: superreview?
Attachment #355285 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #355285 -
Flags: superreview?(dbaron)
Attachment #355285 -
Flags: superreview?
Attachment #355285 -
Flags: review?(dbaron)
Attachment #355285 -
Flags: review?
Comment 13•16 years ago
|
||
Does test_hover.html really need setTimeout(..., 100) rather that setTimeout(..., 0) ?
Assignee | ||
Comment 14•16 years ago
|
||
with 100 it was more reliable if I accidentally moved mouse a bit.
Comment 15•16 years ago
|
||
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.
Comment 16•16 years ago
|
||
And with test_hover.html, could you make sure that the test still fails if you comment out the FlushPendingNotifications call in PresShell::DispatchSynthMouseMove ?
Assignee | ||
Comment 17•16 years ago
|
||
The test fails if PresShell::DispatchSynthMouseMove is commented out.
Comment 18•16 years ago
|
||
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.
Attachment #355285 -
Flags: superreview?(dbaron)
Attachment #355285 -
Flags: superreview+
Attachment #355285 -
Flags: review?(dbaron)
Attachment #355285 -
Flags: review+
Updated•16 years ago
|
Attachment #355285 -
Flags: approval1.9.1?
Assignee | ||
Comment 19•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 20•16 years ago
|
||
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
Comment 21•16 years ago
|
||
This fixed Windows bug 460620 (one more arument to have it on branch)
Comment 22•16 years ago
|
||
This should go into 1.9.1 -- IMHO!
/be
Assignee | ||
Comment 23•16 years ago
|
||
But not if there are regressions :( Bug 472730.
Depends on: 472730
Comment 24•16 years ago
|
||
What about just doing an async event instead of using a scriptrunner? So just post the runnable into the event queue?
Assignee | ||
Comment 25•16 years ago
|
||
Yeah, need to try that.
Assignee | ||
Comment 26•16 years ago
|
||
Unfortunately that doesn't work.
Updated•16 years ago
|
Version: unspecified → Trunk
Updated•16 years ago
|
Flags: wanted1.9.1? → wanted1.9.1+
Comment 27•16 years ago
|
||
What's the performance impact when resizing a window here? Measurable?
Updated•16 years ago
|
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Flags: blocking1.9.1?
The regression is fixed, right? Any reason not to take this on 1.9.1?
Comment 29•16 years ago
|
||
Maybe bug 473805?
Flags: blocking1.9.1?
Comment 30•16 years ago
|
||
smaug, do you think we should wait on bug 473805 to land this on 1.9.1?
Assignee | ||
Comment 31•16 years ago
|
||
Yes.
Comment 32•16 years ago
|
||
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.
Attachment #355285 -
Flags: approval1.9.1?
Updated•15 years ago
|
Whiteboard: [parity-ie] [parity-chrome] [parity-safari] → [parity-ie] [parity-chrome] [parity-safari][needs 191 approval after bug 473805]
Updated•15 years ago
|
Flags: wanted1.9.1.x?
Updated•15 years ago
|
status1.9.1:
--- → ?
Flags: wanted1.9.1.x?
Updated•15 years ago
|
Updated•15 years ago
|
Whiteboard: [parity-ie] [parity-chrome] [parity-safari][needs 191 approval after bug 473805] → [parity-ie] [parity-chrome] [parity-safari]
Updated•15 years ago
|
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.
Description
•