413336 - listbox scrolling behaves badly when flexed and followed by a line-wrapped label

Status: NEW

(Core :: XUL, defect, P5)

Description

[Dave Townsend [:mossop]]

This is the real cause of bug 354527 but I want to spin this off separately in the hope of finding a layout fix whereas the that bug I can propose workarounds for that particular situation.

This bug affects the listbox element where a scrollbar must be displayed, where the listbox is set to flex and where the listbox is followed by a label that is long enough to wrap.

See the attached test case and resize the browser window so the label wraps and the listbox has a scrollbar. Now try to scroll to the bottom of the listbox. You'll should see a cut off item at the bottom or the scrollbar will appear to be able to scroll further than you can push it, it may even bounce down and up when you try like that. If you don't see that try resizing the height of the browser again.

Comment 1

[Dave Townsend [:mossop]]

Attachment: testcase

Comment 2

[Dave Townsend [:mossop]]

I'm trying to dig into this with a debugger but my layout knowledge is not so much up to the task.

What I have discovered is that the listbox seems to get confused over how large an area i has to display in. When trying to scroll down the last bit a cycle of layouts occur and in each layout the return of GetAvailableHeight varies, in my case by 840 The top of the stack of this is:

#0 0x12b1fc91 in nsListBoxBodyFrame::VisibilityChanged at nsListBoxBodyFrame.cpp:452
#1 0x1297fe94 in nsGfxScrollFrameInner::SetScrollbarVisibility at nsGfxScrollFrame.cpp:2614
#2 0x12980044 in nsXULScrollFrame::AddRemoveScrollbar at nsGfxScrollFrame.cpp:1982
#3 0x12980216 in nsXULScrollFrame::AddVerticalScrollbar at nsGfxScrollFrame.cpp:1928
#4 0x129839bd in nsXULScrollFrame::Layout at nsGfxScrollFrame.cpp:2202
#5 0x12983f2c in nsXULScrollFrame::DoLayout at nsGfxScrollFrame.cpp:1203
#6 0x12ae9a6f in nsIFrame::Layout at nsBox.cpp:561

On the first layout after clicking down the height comes to 8520 which appears to be correct. The next it comes out as 9360. This larger value causes the listbox to believe it can display more items than it can and so scrolls itself up one too many.

Interestingly in the first call the height of aScrollAreaSize in AddVerticalScrollbar is 9360 and the second is 8520

Comment 3

[Dave Townsend [:mossop]]

Right and of course the larger height is the height the listbox would have if the label below it wasn't wrapped so I guess the issue is with it laying out the listbox assuming that the label is of a certain fixed height and then discovering it isn't so having to cycle through again?

Comment 4

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Scrolling works OK on the branch too, so this is a regression. (Although clicking on the bottom list item causes a scrollbar jump)

Comment 5

[Dave Townsend [:mossop]]

This is technically a regression from bug 120410.

This bug can be solved by making nsListBoxBodyFrame::VisibilityChanged a no-op
but presumably that would break 120410 again. I'm having difficulty figuring
out why we should ever scroll the box based on the vertical scrollbar appearing
or disappearing though.

Comment 6

[[e-mail]]

I think it would probably help if we didn't call VisibilityChanged - I don't think we're actually changing the scrollbar visibility at this point. So far I haven't found anywhere suitable to cache the old visibility.

Comment 7

[Dave Townsend [:mossop]]

What are the chances of getting some traction here, it would really help bug 354527.

Comment 8

[Dão Gottwald [:dao]]

Attachment: testcase

Comment 9

[Jack]

Is this still the case?  I can't test right now, but something tells me this shouldn't still be New, one way or another.

Comment 10

[u415879]

Yes it is, I used the "Remote XUL Manager" add-on to open the testcase, it's not fixed yet.