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

Categories

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

28 Branch
x86_64
Windows 8.1
defect

Tracking

(firefox28 verified, firefox29 verified)

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

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

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

Attachments

(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
Status: NEW → ASSIGNED
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]
patch

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+
https://hg.mozilla.org/integration/fx-team/rev/c068b477d01d
Flags: in-testsuite+
Whiteboard: [beta28] [defect] p=2 → [beta28] [defect] p=2 [fixed-in-fx-team]
Depends on: 957933
https://hg.mozilla.org/mozilla-central/rev/c068b477d01d
Status: ASSIGNED → RESOLVED
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]
https://hg.mozilla.org/releases/mozilla-aurora/rev/2821312215cb
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.