Closed Bug 952297 Opened 6 years ago Closed 6 years ago

Left scroll arrow in the Metro tab strip is visible even when tab strip is not scrollable


(Firefox for Metro Graveyard :: Theme, defect, P2)

28 Branch
Windows 8.1


(firefox28 verified, firefox29 verified)

Firefox 29
Tracking Status
firefox28 --- verified
firefox29 --- verified


(Reporter: mbrubeck, Assigned: mbrubeck)



(Keywords: polish, regression, Whiteboard: [beta28] [defect] p=2)


(1 file, 1 obsolete file)

When I open up Metro Firefox and use a mouse, the left arrow button from the <arrowscrollbox> in the tab strip is visible and enabled, even if the box is not scrollable because there is only one tab.

I suspect a regression from bug 936635.
Whiteboard: [triage] → [release28] [defect] p=0
Taking this for the current iteration; p=2.
Assignee: nobody → mbrubeck
Whiteboard: [release28] [defect] p=0 → [release28] [defect] p=2
Blocks: metrov1it22
No longer blocks: metrov1backlog
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [release28] [defect] p=2 → [block2828] [defect] p=2
Whiteboard: [block2828] [defect] p=2 → [beta28] [defect] p=2
Attached patch patch (obsolete) — Splinter Review
We were setting "display: visible !important" on the left arrow in precise mode, which overrode the [collapsed] attribute set by the arrowscrollbox code.

This patch instead just overrides the display to "hidden" when they would otherwise be collapsed.
Attachment #8356599 - Flags: review?(sfoster)
Comment on attachment 8356599 [details] [diff] [review]

Can you include an update the browser_tab_container.js test from bug 952663 to turn its todos() into is() and confirm the tests pass with this change?
Attachment #8356599 - Flags: review?(sfoster) → feedback+
Attached patch patch v2Splinter Review
The tests are now passing after some minor tweaks.

I had to use a hard-coded timeout because the arrowscrollbox animated scrolling code uses the same hard-coded timeout.  I'll file a follow-up bug and patch tomorrow to make arrowscrollbox fire an event we can listen to instead.
Attachment #8356599 - Attachment is obsolete: true
Attachment #8357001 - Flags: review?(sfoster)
Comment on attachment 8357001 [details] [diff] [review]
patch v2

Review of attachment 8357001 [details] [diff] [review]:

Looks good and works for me.
Attachment #8357001 - Flags: review?(sfoster) → review+
Flags: in-testsuite+
Whiteboard: [beta28] [defect] p=2 → [beta28] [defect] p=2 [fixed-in-fx-team]
Depends on: 957933
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [beta28] [defect] p=2 [fixed-in-fx-team] → [beta28] [defect] p=2
Target Milestone: --- → Firefox 29
Comment on attachment 8357001 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 936635

User impact if declined: Cosmetic glitch in Metro tab bar when using a mouse.

Testing completed (on m-c, etc.): On m-c since 01-08, includes automated test.

Risk to taking this patch (and alternatives if risky): Low-risk, Metro-only CSS change.

String or IDL/UUID changes made by this patch: None.
Attachment #8357001 - Flags: approval-mozilla-aurora?
Depends on: 952663
Attachment #8357001 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [beta28] [defect] p=2 → [beta28] [defect] p=2 [uplift note: depends on bug 952663]
Whiteboard: [beta28] [defect] p=2 [uplift note: depends on bug 952663] → [beta28] [defect] p=2
For iteration #22, verified as fixed with latest Nightly and Aurora on Win 8.1 64-bit.
You need to log in before you can comment on or make changes to this bug.