Last Comment Bug 651468 - Panning through the add-ons manager doesn't stop at the end of the screen
: Panning through the add-ons manager doesn't stop at the end of the screen
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: ARM Android
: P1 normal (vote)
: Firefox 6
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
Depends on:
Blocks: 641426
  Show dependency treegraph
 
Reported: 2011-04-20 05:29 PDT by Anna (Waverley)
Modified: 2011-07-22 12:49 PDT (History)
11 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.96 KB, patch)
2011-05-20 11:56 PDT, Matt Brubeck (:mbrubeck)
roc: review+
Details | Diff | Splinter Review
patch for checkin (1.97 KB, patch)
2011-05-22 07:35 PDT, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Splinter Review

Description Anna (Waverley) 2011-04-20 05:29:04 PDT
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 Tony Chung [:tchung] 2011-04-20 11:35:51 PDT
regression
Comment 2 Tony Chung [:tchung] 2011-04-20 11:45:46 PDT
Works 4/18, broken 4/19.

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2e5e4197b9db&tochange=bffd5f9dae2f
Comment 3 Aakash Desai [:aakashd] 2011-04-26 09:35:23 PDT
The regression window was found in the previous comment; taking out the regressionwindow-wanted keyword
Comment 4 Matt Brubeck (:mbrubeck) 2011-04-28 09:58:39 PDT
I don't see an obvious regression candidate; I'll start bisecting within the regression range from comment 2.
Comment 5 Matt Brubeck (:mbrubeck) 2011-04-28 13:56:01 PDT
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
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-04-28 17:45:11 PDT
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.
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-04-28 17:45:50 PDT
... or something like that.
Comment 8 Matt Brubeck (:mbrubeck) 2011-04-28 20:49:41 PDT
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.
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2011-04-28 21:15:24 PDT
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.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-04-28 22:20:57 PDT
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.
Comment 11 Matt Brubeck (:mbrubeck) 2011-05-20 11:56:27 PDT
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?
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-21 05:31:29 PDT
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.
Comment 13 Matt Brubeck (:mbrubeck) 2011-05-21 18:29:03 PDT
Comment on attachment 534047 [details] [diff] [review]
patch

Pushed to Try: http://tbpl.mozilla.org/?tree=Try&rev=50dcf61415d0
Comment 14 Matt Brubeck (:mbrubeck) 2011-05-22 07:35:39 PDT
Created attachment 534297 [details] [diff] [review]
patch for checkin

No changes except to add a commit message. This passed Try on all platforms.
Comment 15 Daniel Holbert [:dholbert] 2011-05-22 09:58:24 PDT
http://hg.mozilla.org/projects/cedar/rev/b0ad98f70c91
Comment 16 Matt Brubeck (:mbrubeck) 2011-05-23 08:47:27 PDT
http://hg.mozilla.org/mozilla-central/rev/b0ad98f70c91
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-23 18:32:38 PDT
Thanks!
Comment 18 Anna (Waverley) 2011-05-25 02:10:59 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.