Panning through the add-ons manager doesn't stop at the end of the screen

VERIFIED FIXED in Firefox 6

Status

Fennec Graveyard
General
P1
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Anna (Waverley), Assigned: mbrubeck)

Tracking

({regression})

Trunk
Firefox 6
ARM
Android
regression

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110420 Firefox/6.0a1 Fennec /6.0a1 

Device: HTC Desire Z (Android 2.2)

Steps to Reproduce:
1. Load Fennec.
2. Slide to the left to open the controls sidebar
3. Tap on the browser controls button
4. Tap on the Add-ons manager button
5. Pan down till the end of the screen (till you see the "Browse Site" and "See More Add-ons" buttons)

Actual Results:
Panning through the add-ons manager doesn't stop at the end of the screen. Scrolling continues with a blank display. Please see the video: http://www.youtube.com/user/qaioana#p/u/0/pMxpPRBK4o0

Expected Results:
Panning through the add-ons manager should stop at the end of the screen.

Comment 1

6 years ago
regression
tracking-fennec: --- → ?

Comment 2

6 years ago
Works 4/18, broken 4/19.

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2e5e4197b9db&tochange=bffd5f9dae2f
Priority: -- → P1
Assignee: nobody → mbrubeck
tracking-fennec: ? → ---
Keywords: regressionwindow-wanted
Whiteboard: [fennec-6]
The regression window was found in the previous comment; taking out the regressionwindow-wanted keyword
Keywords: regressionwindow-wanted
(Assignee)

Comment 4

6 years ago
I don't see an obvious regression candidate; I'll start bisecting within the regression range from comment 2.
(Assignee)

Comment 5

6 years ago
The first bad revision is http://hg.mozilla.org/mozilla-central/rev/90e46b3e8e6f

changeset:   71013:90e46b3e8e6f
user:        Robert O'Callahan <robert@ocallahan.org>
date:        Tue Apr 19 15:07:23 2011 +1200
summary:     Bug 641426. Part 5: Avoid operator== where possible to distinguish between 'equal edges' and 'equal areas' for rectangles. r=dbaron,sr=cjones
Blocks: 641426
status-firefox5: --- → unaffected
Keywords: regression
There's probably an element in your XUL with zero width but some height which didn't used to be counted as part of the scrollable area, but now (correctly) is, making the scrollable height larger than it should be.
... or something like that.
(Assignee)

Comment 8

6 years ago
The extra height appears only after the add-ons are loaded, and inserted as richlistitems into the add-on list.

These richlistitems use XBL bindings from mobile/chrome/content/binding/extensions.xml.  The bindings contain anonymous content with class="show-on-select" which is set to "visibility:collapse" in mobile/themes/core/platform.css.

If I comment out the "visibility:collapse" then the content is all visible and matches the height of the container.  So it looks like the collapsed content is being included for some reason when the container height is calculated.
At one time, I thought "collapse" just set the width and height to zero. Maybe that has changed (or I misremembered) and now only the width is set to zero.
A simple testcase doesn't reproduce the problem for me :-(.

You might want to try flipping the changes to layout/xul/base/src/nsSprocketLayout.cpp in http://hg.mozilla.org/mozilla-central/rev/90e46b3e8e6f to IsEqualInterior instead of IsEqualEdges.
tracking-fennec: --- → 6+
Whiteboard: [fennec-6]
tracking-fennec: 6+ → 7+
(Assignee)

Comment 11

6 years ago
Created attachment 534047 [details] [diff] [review]
patch

Sorry, this bug slipped off my radar for a while.  I spent some time a few weeks ago trying to construct a reduced test case, but hadn't got anything that reproduced the problem without including most of the Fennec chrome.

(In reply to comment #10)
> You might want to try flipping the changes to
> layout/xul/base/src/nsSprocketLayout.cpp in
> http://hg.mozilla.org/mozilla-central/rev/90e46b3e8e6f to IsEqualInterior
> instead of IsEqualEdges.

This does fix the regression (see attached patch).  Does this help in constructing a minimal test case, or should I keep trying to reduce one based on Fennec?
Attachment #534047 - Flags: review?(roc)
Comment on attachment 534047 [details] [diff] [review]
patch

Review of attachment 534047 [details] [diff] [review]:
-----------------------------------------------------------------

Let's just take this. This is essentially a very targeted backout.
Attachment #534047 - Flags: review?(roc) → review+
(Assignee)

Comment 13

6 years ago
Comment on attachment 534047 [details] [diff] [review]
patch

Pushed to Try: http://tbpl.mozilla.org/?tree=Try&rev=50dcf61415d0
(Assignee)

Comment 14

6 years ago
Created attachment 534297 [details] [diff] [review]
patch for checkin

No changes except to add a commit message. This passed Try on all platforms.
Attachment #534047 - Attachment is obsolete: true
Attachment #534297 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/projects/cedar/rev/b0ad98f70c91
Keywords: checkin-needed
Whiteboard: fixed-in-cedar
(Assignee)

Comment 16

6 years ago
http://hg.mozilla.org/mozilla-central/rev/b0ad98f70c91
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → Firefox 6
Thanks!
(Reporter)

Comment 18

6 years ago
VERIFIED FIXED on:

Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110524 Firefox/6.0a1 Fennec/6.0a1 

Device: HTC Desire Z (Android 2.2)
Status: RESOLVED → VERIFIED
tracking-fennec: 7+ → 6+
status-firefox6: --- → fixed
You need to log in before you can comment on or make changes to this bug.