Steps to reproduce: 1. Open Customize Window. 2. Put an icon on the toolbar and close the customize window. 3. After reopening you should see it. Regression between 1.9a1_2006031504 and 1.9a1_2006031521. http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2006-03-15+03%3A00&maxdate=2006-03-15+21%3A00 No idea what fix might have caused this but Bug 192767 is about scrolling.
The white scroll bar disappears when I choose "Restore Default Set".
Step two is unnecessary. You just need to open the customize window twice; the second time it will have the problem.
This occurs on Mac OS X as well -> All/All.
OS: Windows XP → All
Hardware: PC → All
Created attachment 249826 [details] Simple xul testcase. Demonstrates the phantom horizontal scrollbar with a xul box with overflowed (and flex=1) contents. Regression range for Mac is between 20060315-05 and 20060316-05, which fits the check-in for bug 192767 being the cause (though this clearly isn't an rtl/bidi issue).
(In reply to comment #5) I forgot to note that resizing the browser window so that the horizontal scrollbar is partially covered forces a redraw which removes the bad scrollbar. So the code for redrawing the box seems to know something that the initially executed code doesn't.
Created attachment 249987 [details] [diff] [review] Not a true fix, but a temp fix that highlights the code problem I can confirm that attachment 215135 [details] [diff] [review] of bug 192767 caused this regression. I've narrowed it down to one specific section of code. The attached patch is actually a reversal of that section of the original patch. While I don't expect it's a legitimate fix, I hope it'll help dbaron work out what's wrong. Basically... when overflow is auto and the horizontal case is tested, the current method of calculating the scrolledContentSize width yields a value that is sometimes greater than the original method (which used GetScrolledSize() ). This means that |scrolledContentSize.width > scrollAreaRect.width| returns true in cases that it originally wouldn't have. Actually, as far as I can tell, it looks like the older method gave a width that was identical to the scrollAreaRect.width. Hope this helps.
In the case where this fixes the problem, what are: * the old scrollAreaRect? * the new scrollAreaRect? * scrolledRect? * scrolledContentSize? (You should be able to get all of that from a pair of printfs right before and after your inserted line.)
12 years ago
(In reply to comment #8) I hope that the width and height values were what you were after (which were always the same as XMost() and YMost() for the nsRects). Using the xul test case (attachment 249826 [details]), the values I got were: scrolledRect: width = 2250 height = 3885 scrollAreaRect: width = 2250 height = 2250 scrolledContentSize: width = 2250 height = 3885 Old scrolledContentSize: width = 2025 height = 4403 Where scrolledContentSize is the one using the currently used method, and the "Old" one is the original that used mInner.GetScrolledSize(). The old width is 225 less than the new one, while the height is 518 greater. Calling up the customize toolbar window (on Mac), I got: 1st time (no difference): scrolledRect: width = 6525 height = 5280 scrollAreaRect: width = 6525 height = 4078 scrolledContentSize: width = 6525 height = 5280 Old scrolledContentSize: width = 6525 height = 5280 2nd and subsequent times (difference): scrolledRect: width = 6750 height = 5280 scrollAreaRect: width = 6750 height = 4078 scrolledContentSize: width = 6750 height = 5280 Old scrolledContentSize: width = 6525 height = 5280 Again, the old method gave a width that was 225 less than the new one. The heights are identical. I loaded a local copy of the test case, and changed the width and height of the box. Regardless of the values that I used, I found that the "old" width was always exactly 225 lower than the current one, unless (a) the height was increased until there was no overflow, in which case the "old" width value increased by 225 to be the same as the current one, or (b) the width was squeezed until the horizontal overflow was legitimate. In the latter case, the old and current width values did vary a little (by 37) when the box was set to a width of 5em, but the values were identical once the box was set to 4em or less (in which case the scrollbar was too narrow for a scroll button to be drawn). I don't know what's magic about the value of 225. It happens to be the square of 15, which is the exact height of the scrollbar in pixels, but I assume that's a coincidence (?!?).
I see it now. And it's nothing to do with the method used for getting the rectangle dimensions. It's due to the vertical scrollbar being added here: http://lxr.mozilla.org/seamonkey/source/layout/generic/nsGfxScrollFrame.cpp#2179 Adding the vertical scrollbar reduces the scrolledContentSize width by 225. In the original code, the scrolledContentSize was queried again at the start of the horizontal scrollbar check (which is below the vertical scrollbar code), so it got the new width. But with the current code, the dimensions are only calculated before the vertical scroll is checked for possible addition or removal, so the width can be wrong. Recalculating the scrolledContentSize after the vertical scrollbar code fixes it. I presume the |if (needsLayout)| code is intended to do this already?
Created attachment 250179 [details] [diff] [review] patch Right. Thanks for figuring that out. Here's a patch to do that. This restores the refetching code that existed before the patch to bug 192767 by scoping the variables to a scope in which they're still valid (well, not exactly, since the call to LayoutScrollArea is near the end of the first scope).
12 years ago
(In reply to comment #11) Thanks David. The patch seems to work well for me.
Checked in to trunk. Thanks again for figuring it out.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
VERIFIED Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a2pre) Gecko/20070104 Minefield/3.0a2pre ID:2007010404 [cairo]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.