Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Vertical scrollbar missing on specific sites since upgrading to Firefox 12

RESOLVED FIXED in Firefox 13

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Tobbi, Assigned: Mats Palmgren (vacation - back in August))

Tracking

({regression})

Trunk
mozilla15
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox13+ verified, firefox14+ verified)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
A user in the Support forums reported that the scrollbar is missing on a certain site:
http://www.microsoft.com/learning/en/us/Exam.aspx?ID=70-659#tab2

I can reproduce the issue with the latest trunk. On the Microsoft site, there doesn't seem to be 'overflow-y: hidden' anywhere so I doubt this is actually an issue with the site. 

In addition, the site works correctly in Microsoft Internet Explorer.

Thread for reference:
https://support.mozilla.org/en-US/questions/926245
(Reporter)

Updated

5 years ago
Keywords: regression, regressionwindow-wanted
(Reporter)

Updated

5 years ago
(Reporter)

Comment 1

5 years ago
Last good nightly: 2012-01-17
First bad nightly: 2012-01-18

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=34572943a3e4&tochan
ge=f4049f65efc6
Keywords: regressionwindow-wanted
(Reporter)

Comment 2

5 years ago
Proper Pushlog URL:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=34572943a3e4&tochange=f4049f65efc6
Keywords: testcase-wanted

Comment 3

5 years ago
Created attachment 619291 [details]
sample html

Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/0b4c58200e3a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120117 Firefox/12.0a1 ID:20120117042508
Fails:
http://hg.mozilla.org/mozilla-central/rev/ff1bedd7d463
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120117 Firefox/12.0a1 ID:20120117072408
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b4c58200e3a&tochange=ff1bedd7d463


Regression window(m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/7ea3311b5e6c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120116 Firefox/12.0a1 ID:20120116145408
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/3077b729dfd5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120116 Firefox/12.0a1 ID:20120116153908
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7ea3311b5e6c&tochange=3077b729dfd5


Last good:
L:\trunk\2012\01\bin inbound 16-Jan-2012 1538-5 804c80adda0a
First bad:
L:\trunk\2012\01\bin inbound 16-Jan-2012 1538-6 87536f378706

Triggered by:
87536f378706	Mats Palmgren — Bug 665597 - Use saturating calculations when adding the margin to the scrollable overflow rect. part=4/5 r=roc

Updated

5 years ago
Blocks: 665597

Comment 4

5 years ago
about attachment 619291 [details], it works if I change css 
.hide { position: absolute; left: -999999em; top: -999999em; }
instead of
.hide { position: absolute; left: -99999em; top: -99999em; }

I guess that big value of css is a problems.
Thanks Alice, yes you're right about the cause.

-999999em in itself is less than nscoord_MIN so it's clamped in the style system
already, then when calculating the scrollable overflow rect for #body, adding
the 1000px height doesn't make a difference since the overflow rect height is
clamped to nscoord_MAX.  Previously we would end up with nscoord_MAX + 1000px.

I don't think there is a way to fix this properly unless we start using floats
(bug 265084).  We can mitigate this by clamping harder in the style system on
specified values, for example clamping to nscoord_MAX / 2 would still allow
for 8947849px.  That way, adding reasonable values to it during layout
shouldn't trigger clamping unless a bunch of these values are added together
but that seems like it would be rare in practice...
Created attachment 619324 [details] [diff] [review]
wip

something like this...

https://tbpl.mozilla.org/?tree=Try&rev=406b10c4f60b
Duplicate of this bug: 749940
(Reporter)

Updated

5 years ago
Keywords: testcase-wanted
Created attachment 619464 [details] [diff] [review]
fix

I think this is a relatively low-risk fix for this bug,
if indeed we want to try to fix it at all.
What do you think?

https://tbpl.mozilla.org/?tree=Try&rev=9ae39ed8d6a0
Assignee: nobody → matspal
Attachment #619324 - Attachment is obsolete: true
Attachment #619464 - Flags: feedback?(roc)
Attachment #619464 - Flags: feedback?(dbaron)
Attachment #619464 - Flags: feedback?(roc) → feedback+
Attachment #619464 - Flags: feedback?(dbaron) → review?(dbaron)
Comment on attachment 619464 [details] [diff] [review]
fix

>+// CoordStyleClamp clamps to nscoord_MIN/2 .. nscoord_MAX/2 unlike
>+// NSToCoordRoundWithClamp which clamps to nscoord_MIN .. nscoord_MAX.
>+// This is to allow adding/subtracting reasonable values to it during
>+// layout without triggering NSToCoordRoundWithClamp clamping there.

So nscoord_MIN and nscoord_MAX already give us only half the range of 32-bit integers, for exactly this sort of reason.  And now we're restricting it to half again?  I'm not crazy about that idea.

Maybe we should consider doing this nscoord_MIN/2 sort of clamping when unioning rectangles -- just for the x and the y, so that we'd essentially never allow a very small x/y to destroy the useful part of the height data?  Or maybe we should allow the width and height of overflow rects to use the full 32-bit int range?
Blocks: 751278
(In reply to David Baron [:dbaron] from comment #9)
> Maybe we should consider doing this nscoord_MIN/2 sort of clamping when
> unioning rectangles -- just for the x and the y, so that we'd essentially
> never allow a very small x/y to destroy the useful part of the height data?

Makes sense to me.

Updated

5 years ago
Duplicate of this bug: 749699
Attachment #619464 - Attachment is obsolete: true
Attachment #619464 - Flags: review?(dbaron)
Created attachment 623512 [details] [diff] [review]
fix+tests

Make all nsRect Union methods saturating.  If width/height overflows nscoord_MAX then clamp the x/y to nscoord_MIN / 2 and try again.
Attachment #623512 - Flags: review?(roc)
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=23ca42af1058
Attachment #623512 - Flags: review?(roc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c0f00a6a12f
Flags: in-testsuite+
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/8c0f00a6a12f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Probably worth taking on branches considering the risk/benefit.
tracking-firefox13: --- → ?
tracking-firefox14: --- → ?
(In reply to Mats Palmgren [:mats] from comment #16)
> Probably worth taking on branches considering the risk/benefit.

Sounds good - we agree. Please nominate for aurora/beta approval when ready (but before 5/21 to make it into our next beta).
tracking-firefox13: ? → +
tracking-firefox14: ? → +
Comment on attachment 623512 [details] [diff] [review]
fix+tests

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 665597
User impact if declined: missing scrollbars on pages that use excessively large CSS length values
Testing completed (on m-c, etc.): baked on m-c since 2012-05-15
Risk to taking this patch (and alternatives if risky): The patch makes the code slightly more branchy in a relatively hot path, so there's a (low) risk for performance regressions in layout heavy work loads.
String or UUID changes made by this patch: none
Attachment #623512 - Flags: approval-mozilla-beta?
Attachment #623512 - Flags: approval-mozilla-aurora?
Landed on Aurora per verbal approval in comment 17:
https://hg.mozilla.org/releases/mozilla-aurora/rev/5c69f909484c
still waiting for the mozilla-beta tree to open...
status-firefox14: --- → fixed

Updated

5 years ago
Attachment #623512 - Flags: approval-mozilla-beta?
Attachment #623512 - Flags: approval-mozilla-beta+
Attachment #623512 - Flags: approval-mozilla-aurora?
Attachment #623512 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-beta/rev/22cfcc320512
status-firefox13: --- → fixed

Updated

5 years ago
Duplicate of this bug: 758817
Whiteboard: [qa+]

Comment 22

5 years ago
Verified as fixed with the link from comment 0 and the sample.html on:
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0
BuildID: 20120528154913
status-firefox13: fixed → verified

Comment 23

5 years ago
Verified as fixed with the link from comment 0 and the sample.html on:
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0 BuildID: 20120605113340
status-firefox14: fixed → verified
Whiteboard: [qa+]
Blocks: 750323

Comment 24

5 years ago
Maybe I didn't understood what this bug is but could someone help me with this?
http://i50.tinypic.com/e96rg0.jpg
I think it's the same issue you're talking about but I've got on FF 14.0.1 :/
hiryu, that looks like a different problem.  Please file a new bug if you
can reproduce it using a fresh profile.  Thanks.
You need to log in before you can comment on or make changes to this bug.