1005964 - opening page with saved zoom level in background tab can fire resize event inside a flush triggered by the page's script, confusing that page's script

Status: NEW

(Core :: Layout, defect, P4)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

STR:
1. Open http://mollyrocket.com/casey/
2. Hit control-+ a few times to zoom in
3. Middle click any of the blog entry links, or right-click "open in new tab"
4. Switch to new tab.

All the paragraphs will be overlaid on top of eachother.

This does not happen if a link is followed in the same tab; only if it's opened in a new tab.  Also, if a reflow is forced, (by resizing window or opening up the dev tools, say), the page lays out correctly.

Reproduces in 29 and in 32.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Looks like this page is full of script that does the layout itself; every paragraph is absolutely positioned.

I'd presume it has code to handle both initial layout and resizing.  My first guess would be that the code to handle initial layout doesn't deal with certain APIs returning values with fractional pixels.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, the APIs it's using don't return fractional pixels, and I'm really not sure why they're doing what they're doing.  But it looks complicated to debug.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Another explanation just occurred to me, and I realized that a site-side workaround for the bug is to change:

window.onresize=L;L();

at the very end of the script into:

L();window.onresize=L;

(This fixes it for me locally.)


I suspect we're recurring into the resize handler during the script and confusing the script as a result.

Unfortunately I can't reproduce the bug in my debug build, only in a nightly, so I can't debug why.  On the other hand, that might mean I have a patch in my patch queue (probably ancient) that somehow fixes it.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #3)
> Unfortunately I can't reproduce the bug in my debug build, only in a
> nightly, so I can't debug why.  On the other hand, that might mean I have a
> patch in my patch queue (probably ancient) that somehow fixes it.

That appears to be because the per-site persistence of zoom doesn't work for file: URLs in my debug build.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is how we fire the resize event during the script:

#0  PresShell::FireResizeEvent (this=0x504ae80)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:2046
#1  0x00007f23192a64e7 in PresShell::FlushPendingNotifications (
    this=0x504ae80, aFlush=...)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:4003
#2  0x00007f2319297a48 in PresShell::FlushPendingNotifications (
    this=<optimized out>, aType=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:3938
#3  0x00007f2318e2ee60 in nsDocument::FlushPendingNotifications (
    this=0x49a38a0, aType=Flush_Layout)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/nsDocument.cpp:7835
#4  0x00007f2318e62c48 in mozilla::dom::Element::GetPrimaryFrame (
    this=0x5012140, aType=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:1649
#5  0x00007f2318e62c6a in mozilla::dom::Element::GetStyledFrame (
    this=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:505
#6  0x00007f2318fc49b3 in nsGenericHTMLElement::GetOffsetRect (this=0x5012140, 
    aRect=...)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:333
#7  0x00007f231879bde7 in nsGenericHTMLElement::OffsetHeight (
    this=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/html/content/src/nsGenericHTMLElement.h:295
#8  0x00007f2318784182 in mozilla::dom::HTMLElementBinding::get_offsetHeight (
    cx=<optimized out>, obj=..., self=<optimized out>, args=...)
    at /home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/dom/bindings/HTMLElementBinding.cpp:1488



There are a bunch of resizes that happen during the loading process, the last of which is:

#0  nsViewManager::SetWindowDimensions (this=0x5c72470, aWidth=77110, 
    aHeight=43395)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/view/src/nsViewManager.cpp:224
#1  0x00007f231931a473 in nsPresContext::SetFullZoom (this=0x32d1d50, 
    aZoom=1.10000002)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresContext.cpp:1462
#2  0x00007f2319290749 in nsDocumentViewer::SetFullZoom (this=0x3170ce0, 
    aFullZoom=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsDocumentViewer.cpp:2985

(The previous two resizes resized to a slightly larger size, i.e., the same without the zoom applied.)

Then when we get here:

#0  PresShell::ResizeReflowIgnoreOverride (this=0x4d05710, aWidth=77110, 
    aHeight=43395)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:1955
#1  0x00007f2318e16c80 in nsViewManager::DoSetWindowDimensions (
    this=0x5c72470, aWidth=77110, aHeight=43395)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/view/src/nsViewManager.cpp:201
#2  0x00007f2318e16cd8 in nsViewManager::FlushDelayedResize (this=0x5c72470, 
    aDoReflow=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/view/src/nsViewManager.cpp:236

In frame #1, oldDim is the previous (unzoomed) size, so we do an extra reflow, which posts a resize event, which gets flushed in the stack at the top of this comment.  (Without the zoom, we'd have bailed in frame #1.)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Note that this probably means that the performance of zoom persistence is not what it should be.)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, and the final stack continues as:

#3  0x00007f23192a66a5 in PresShell::FlushPendingNotifications (
    this=0x4d05710, aFlush=...)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:4089
#4  0x00007f2319297a48 in PresShell::FlushPendingNotifications (
    this=<optimized out>, aType=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:3938
(and then matches the top stack with a 2 frame offset)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, and I think we ought to fix this by doing something so that we don't have a queued-up resize event at all.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fix works, but I need to write a test.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

While trying to write a test, I discovered that I can't reproduce the original bug in a nightly build anymore!

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This might have been related to the problem found in bug 1279202.