If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

with view source, when I have a horizontal scrollbar, the vertical scrollbar is "too short"

RESOLVED FIXED

Status

()

Core
General
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Unassigned)

Tracking

({dogfood, regression})

Trunk
x86
Mac OS X
dogfood, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

with view source, when I have a horizontal scrollbar, the vertical scrollbar is "too short".

here comes two screen shots to show what I mean.

I'm using my own trunk, debug Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070502 Minefield/3.0a5pre
Created attachment 263636 [details]
screen shot (no horizontal scrollbar)
Created attachment 263637 [details]
screen shot (with horizontal scrollbar) and the problem

Comment 3

11 years ago
For me, it is the vertical scrollbar whose location is wildly off.
crowder:  to clarify:  for mac, too?  and, for view source (or another window?)

Comment 5

11 years ago
Yes, Mac too, but not only the view source window.  This happens to me in every window with horizontal scroll.
STR or testcase? I can't reproduce this.

Comment 7

11 years ago
It happens to me with a build from an hour or so ago while looking at many bugzilla bugs, even just https://bugzilla.mozilla.org/.  The characteristics change sometimes, if I resize the window, also the dropdown for "Target Milestone" on this page is very very messed up, too.  The down-arrow "button" is hundreds of pixels wide.
> The down-arrow "button" is hundreds of pixels wide.

I had never seen that, until just now.  screen shot coming.
Created attachment 263677 [details]
screen shot
Component: General → General
Product: Firefox → Core
QA Contact: general → general
I'm also seeing this, usually the vertical scroll bar. It seems to happen more frequently when the browser has been running for a while. First started for me about a week ago.

usually I get a blank window with the scrollbar on the left.

Updated

11 years ago
Keywords: dogfood, regression
I suspect a problem with scrollbar metrics. The dropdown arrow width is defined in terms of scrollbar metrics. Also, too-wide vertical scrollbar metrics would produce exactly the misplaced scrollbar that I'm seeing. The erraticness suggests problems with uninitialized variables.

Updated

11 years ago
Blocks: 380593
Duplicate of this bug: 380593
The problem is the sizing of NS_THEME_SCROLLBAR_THUMB_VERTICAL (or HORIZONTAL, I guess). Just after the call to ::HIThemeGetTrackPartBounds(&tdi, kControlIndicatorPart, &thumbRect), I have the following result:

(gdb) p thumbRect
$12 = {
  origin = {
    x = 0, 
    y = 3.91198254
  }, 
  size = {
    width = 4074.08203, 
    height = 9.80908925e-45
  }
}

Looks like uninitialized garbage to me! It may have failed because the scrollbar frame has not yet been reflowed:

(gdb) p scrollbarFrame->mRect
$14 = {
  x = 0, 
  y = 0, 
  width = 0, 
  height = 0
}

Here's the corresponding tdi:

(gdb)  p tdi
$15 = {
  version = 0, 
  kind = 0, 
  bounds = {
    origin = {
      x = 0, 
      y = 0
    }, 
    size = {
      width = 0, 
      height = 0
    }
  }, 
  min = 0, 
  max = 0, 
  value = 0, 
  reserved = 1465171967, 
  attributes = 4, 
  enableState = 0 '\000', 
  filler1 = 0 '\000', 
  trackInfo = {
    scrollbar = {
      viewsize = 0, 
      pressState = 56 '8'
    }, 
    slider = {
      thumbDir = 0 '\000', 
      pressState = 0 '\000'
    }, 
    progress = {
      phase = 0 '\000'
    }
  }
}
Created attachment 264817 [details] [diff] [review]
fix

This fixes it for me.
Attachment #264817 - Flags: superreview?
Attachment #264817 - Flags: review?(cbarrett)
Attachment #264817 - Flags: superreview? → superreview?(mats.palmgren)
No longer blocks: 380593

Comment 15

11 years ago
You could avoid doing scrollbarFrame->GetRect() twice by moving your new code down one line and using the scrollbarRect variable.
Comment on attachment 264817 [details] [diff] [review]
fix

Jesse's suggestion is correct. r=cbarrett with that change.
Attachment #264817 - Flags: review?(cbarrett) → review+
Comment on attachment 264817 [details] [diff] [review]
fix

Stealing the sr so we can unbustimicate the tree.  As per IRC, no need to call SizeTo(0,0) there, as we've already done so at the beginning of the method.  sr=dmose with that change.
Attachment #264817 - Flags: superreview?(mats.palmgren) → superreview+
checked in
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
But we don't want GetPrefSize and friends to be dependent on reflow, right?
With the current fix we get a zero min-width before reflow and something
else after.  It seems to me that this could cause incremental reflow bugs
or at least unnecessary reflows?

FWIW, the stack leading up to the problem is:

nsSprocketLayout::Layout
 nsSprocketLayout::PopulateBoxSizes
  nsSliderFrame::GetPrefSize
   nsBoxFrame::GetPrefSize
    nsSprocketLayout::GetPrefSize
     nsBoxFrame::GetPrefSize
      nsBoxFrame::GetMinSize
       nsIFrame::AddCSSMinSize
        nsNativeThemeCocoa::GetMinimumWidgetSize
         scrollbarFrame->GetRect()

Shouldn't we use scrollbarFrame->GetMinSize() there instead?
(In reply to comment #19)
> But we don't want GetPrefSize and friends to be dependent on reflow, right?

Right. But in this case we have no choice because the platform's desired size does depend on the height of the scrollbar. We *have* to return the correct value here or we will have bugs because the platform's scrollbar thumb isn't where our thumb frame is. Put it another way, the problem here is that Aqua doesn't obey our reflow rules...

> With the current fix we get a zero min-width before reflow and something
> else after.  It seems to me that this could cause incremental reflow bugs
> or at least unnecessary reflows?

Possibly. Hopefully the minimum width of the thumb doesn't really matter because the buttons will give us a non-varying minimum width anyway.

> FWIW, the stack leading up to the problem is:
> 
> nsSprocketLayout::Layout
>  nsSprocketLayout::PopulateBoxSizes
>   nsSliderFrame::GetPrefSize
>    nsBoxFrame::GetPrefSize
>     nsSprocketLayout::GetPrefSize
>      nsBoxFrame::GetPrefSize
>       nsBoxFrame::GetMinSize
>        nsIFrame::AddCSSMinSize
>         nsNativeThemeCocoa::GetMinimumWidgetSize
>          scrollbarFrame->GetRect()
> 
> Shouldn't we use scrollbarFrame->GetMinSize() there instead?

I don't see how that would help.

Maybe we should modify scrollbar reflow so that on Mac, at least, the thumb size is not taken into account when determining the pref or min size of the slider.
(In reply to comment #20)
> Maybe we should modify scrollbar reflow so that on Mac, at least, the thumb
> size is not taken into account when determining the pref or min size of the
> slider.

While you're thinking about changing the way scrollbars work on the Mac, bug 380185 is worth a look.
You need to log in before you can comment on or make changes to this bug.