708062 - Page zoom use at Twitter causes feeds to disappear leaving only the page background visible

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[DB Cooper]

Login to https://twitter.com/ and increase the page zoom one increment with Ctrl++. Feeds disappear leaving only the page background image visible. 

Returning to the default zoom with Ctrl+0 does not make page elements visible again. Nor does any other page zoom operation. 

Refreshing the page does make all elements visible again.

Repeated in safe mode. Confirmed by mozillazine builds forum member here:

http://forums.mozillazine.org/viewtopic.php?p=11534429#p11534429

Comment 1

[jorge alves]

Resizing the window also restores the page content.

Comment 2

[Alice0775 White]

Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/cb70391c86d9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111205 Firefox/11.0a1 ID:20111205043819
Fails:
http://hg.mozilla.org/mozilla-central/rev/fafaf614791f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111205 Firefox/11.0a1 ID:20111205100918
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cb70391c86d9&tochange=fafaf614791f

Regression window(m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/03420089b4af
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111205 Firefox/11.0a1 ID:20111205030218
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d991d9770292
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111205 Firefox/11.0a1 ID:20111205045018
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=03420089b4af&tochange=d991d9770292

Suspecrd:
Bug 699351

Comment 3

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

Attachment: Simplified testcase

Fairly minimal testcase

Comment 4

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

In this testcase, after fixing bug 699351, the viewport overflow area is set to its correct value, which includes horizontal overflow due to the fixed-pos element. This causes the root view for that document to be wider than the viewport. Then nsPresContext::SetFullZoom fetches those view dimensions, adjusts them for zooming and uses them to reset nsPresContext::SetVisibleArea() --- to a larger value than it should have. That incorrect width is then used for the viewport width when we reflow the document.

So basically the bug is that the root view's bounds include the viewport's overflow area, but nsPresContext::SetFullZoom expects it to be the viewport's normal bounds.

Comment 5

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

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> Then nsPresContext::SetFullZoom fetches those view
> dimensions, adjusts them for zooming and uses them to reset
> nsPresContext::SetVisibleArea()

That last step happens via nsViewManager::SetWindowDimensions calling nsViewManager::DoSetWindowDimensions calling PresShell::ResizeReflow.

Multiple places assume that the root view bounds match the viewport size. So I think the best way to fix this is to restore that invariant by stopping the root view bounds from including the viewport overflow area.

Comment 6

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

Attachment: fix

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 579931 [details] [diff] [review]
fix

>+  // Always use boundsRelativeToTarget here, not desiredSize.GetVisualOverflowArea(),
>+  // because for root frames (where they could be different, since root frames
>+  // are allowed to have overflow) ...

They can't be different for any other reflow roots? Would making this change conditional on the reflow target being a root frame be better?

Comment 8

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

(In reply to Timothy Nikkel (:tn) from comment #7)
> Comment on attachment 579931 [details] [diff] [review] [diff] [details] [review]
> fix
> 
> >+  // Always use boundsRelativeToTarget here, not desiredSize.GetVisualOverflowArea(),
> >+  // because for root frames (where they could be different, since root frames
> >+  // are allowed to have overflow) ...
> 
> They can't be different for any other reflow roots? Would making this change
> conditional on the reflow target being a root frame be better?

Other reflow roots are not allowed to have visual overflow different from the border-box ... see assertions above this code.

Comment 9

[Timothy Nikkel (:tnikkel)]

Comment on attachment 579931 [details] [diff] [review]
fix

Oh, silly me, the answer was right in front of me.

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/170cd2848f17

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/170cd2848f17