Closed Bug 965550 Opened 10 years ago Closed 10 years ago

Tab stip doesn't touch scroll if last tab is partially visible


(Firefox for Metro Graveyard :: App Bar, defect, P1)

Windows 8.1


(firefox28 verified, firefox29 verified, firefox30 verified, b2g-v1.3 fixed)

Firefox 30
Tracking Status
firefox28 --- verified
firefox29 --- verified
firefox30 --- verified
b2g-v1.3 --- fixed


(Reporter: rsilveira, Assigned: rsilveira)



(Whiteboard: [release28] p=2 s=it-30c-29a-28b.2 r=ff28)


(1 file)

1. Open as many tabs as necessary to have the last one partially visible (that's 6 tabs on the X1 carbon and 5 tabs on the surface pro)
2. Scroll the tab strip to the beginning and select the first tab.
3. After the tab stip dismisses, being it up again.

Now you can't touch scroll the tab strip. All tabs are still selectable. Adding a new tab fixes it.
Whiteboard: [triage] [defect] p=0
Whiteboard: [triage] [defect] p=0 → [release28] [defect] p=0
Assignee: nobody → rsilveira
Whiteboard: [release28] [defect] p=0 → [release28] [defect] p=2
I was trying to find any of our front-end code that could be related to this with no luck. It happens in -metrodesktop too, which makes it easier to debug. I'm starting to think it could be something deeper.
Priority: -- → P1
QA Contact: jbecerra
Whiteboard: [release28] [defect] p=2 → [release28] [defect] p=2 s=it-30c-29a-28b.1
Target Milestone: --- → Firefox 28
Whiteboard: [release28] [defect] p=2 s=it-30c-29a-28b.1 → [release28] p=2 s=it-30c-29a-28b.1 r=ff28
Target Milestone: Firefox 28 → ---
Assignee: rsilveira → nobody
Whiteboard: [release28] p=2 s=it-30c-29a-28b.1 r=ff28 → [release28] p=2 r=ff28
Assignee: nobody → rsilveira
Whiteboard: [release28] p=2 r=ff28 → [release28] p=2 s=it-30c-29a-28b.2 r=ff28
TouchModule check for dragability[1] compares the target bounding rect with the scroller size. The overgrown scroll arrows[2] are not discounted from the bounding rect, thus it will only scroll if the scroller width is larger than the tabs viewable are plus both scroll buttons (64px each).

We used to collapse the arrow buttons when in imprecise mode but it caused it's own issues (bug 8346255).I'll try to see if I can get a better solution. 

[1] -
[2] -
Attached patch Patch v1Splinter Review
Adding padding to the size of the arrow buttons and compensating with negative margin fixes it. This is a very special case where we resize the arrow buttons on a scrollbox, we can investigate a more generic fix if it comes back somewhere. It's very targeted and low risk for uplift too.
Attachment #8381545 - Flags: review?(mbrubeck)
Attachment #8381545 - Flags: review?(mbrubeck) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
For testing and verification.  Reopen if any defects found.
Flags: needinfo?(kamiljoz)
Went through the verification process with the latest Nightly build:

I reproduced the original issue using the following build before going through verification:

- Went through the original test case from comment #0 and ensured it was working correctly (couldn't reproduce the original issue)
- Opened over 15 different tabs and ensured that I can swipe and scroll through the Tab App Bar
- Ensured that all the tabs under the Tab App Bar are selectable
- Ensured that you can close all the opened tab's using the "X"
- Ensured that you can scroll using the arrows on each side of the Tab App Bar (quickly clicking through & clicking and holding)
- Ensured that you can scroll through the tabs using the mouse wheel
- Went through all of the above test cases using different variations of snapped view
- Tested with both the X1 Carbon and the Surface Pro 2
Flags: needinfo?(kamiljoz)
Comment on attachment 8381545 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): It was uncovered by bug 936635
User impact if declined: Tab strip will not be touch scrollable if you have the last tab partially visible.
Testing completed (on m-c, etc.): on m-c since 2/27, QA tested - see comment 7.
Risk to taking this patch (and alternatives if risky): Very low, CSS change impacting Metro only.
String or IDL/UUID changes made by this patch: none.
Attachment #8381545 - Flags: approval-mozilla-beta?
Attachment #8381545 - Flags: approval-mozilla-aurora?
Attachment #8381545 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Changing the issue back to resolved until the other channels have been tested and verified.
Closed: 10 years ago10 years ago
Went through the verification process using the following build:

- Went through the original test case from comment #0
- Went through the test cases added in comment #7 (used both X1 Carbon/Surface Pro 2)
Comment on attachment 8381545 [details] [diff] [review]
Patch v1

Will take this on Beta even though it doesn't meet the typical criteria for this stage. It's low risk and Metro only so we'll go for it to help Metro be its best on first public release.
Attachment #8381545 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: verifyme
Verified as fixed with Firefox 28 beta 9 on Dell XPS 12 ultrabook with Windows 8 64-bit.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.