551463 - Toolbar content scrolls after toolbarbutton sub-menu is opened

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Alice0775 White]

Attachment: testcase

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100310 Minefield/3.7a3pre ID:20100310040439
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a3pre) Gecko/20100310 Minefield/3.7a3pre ID:20100310040439

Toolbar content scrolls after toolbarbutton sub-menu is opened

Steps to Reproduce:
1. Open testcase2
2. Scroll down
3. Click Button22, Button36 etc

Actual Results:
 the upper hierarchy toolbar scrolls when a menu opened.

Expected Results:
 should not scroll

regression window:

works:
http://hg.mozilla.org/mozilla-central/rev/5dca6c28cca5
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100111 Minefield/3.7a1pre ID:20100111152135

fails:
http://hg.mozilla.org/mozilla-central/rev/a43e2f7eda8f
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100112 Minefield/3.7a1pre ID:20100112034523

pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5dca6c28cca5&tochange=a43e2f7eda8f

candidate bug:
Bug 526394

Comment 1

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

Here we have an nsHTMLScrollFrame inside a XUL box. When the popup is shown, we flush pending reflows for the XUL document. XUL calls nsFrame::RefreshSizeCache on a subtree including the nsHTMLScrollFrame; this finds the preferred width by reflowing the subtree with infinite available width. With that width, all the content fits on one line. nsHTMLScrollFrame decides that means we need to scroll all the way up to the top. Later we get another reflow to give us the correct height, but by then it's too late, the old scroll position is lost.

Comment 2

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

I think the reason this worked before is that before, we stored the scroll position in a variable in nsGfxScrollFrameInner and didn't clamp it based on the frame size until we reached a post-reflow callback. Now we're clamping oldScrollPosition in nsHTMLScrollFrame::Reflow by calling nsGfxScrollFrameInner::ClampAndRestrictToDevPixels.

Comment 3

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

Attachment: fix

Comment 4

[Mats Palmgren (inactive)]

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

r=mats

While you're here, please remove the comments on SrcollToImpl
being destructive on line 1348/1354:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#1347
because it's not true after bug 526394 (I hope).

Comment 5

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

OK

Comment 6

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

oops

Comment 7

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

http://hg.mozilla.org/mozilla-central/rev/6f80d1746d80