Closed Bug 1135589 Opened 5 years ago Closed 5 years ago

The focusring for the controls in reader mode doesn't match the visible borders of the buttons

Categories

(Firefox :: General, defect, P4)

38 Branch
defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 39
Iteration:
39.3 - 30 Mar
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: avaida, Assigned: jaws)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Reproducible on:
Nightly 38.0a1 (2015-02-22)

Affected platforms:
Windows 8.1 (x86), Ubuntu 14.04 (x64)

Steps to reproduce:
1. Launch Firefox with a clean profile.
2. Enable Reader Mode from about:config by setting 'reader.parse-on-load.enabled' to true. Restart the browser.
3. Open an article from the web - (e.g.) http://www.bbc.com/news/world-australia-31579804
4. Enter Reader Mode by clicking the associated button from the Location Bar.
5. Click any of the buttons available in the sidebar.

Expected result:
Clicking the page styling button [Aa] for example, brings up the styling options pane without any UI issues. 

Actual result:
Once clicked, a selection rectangle is placed on the button in question.

Notes:
- Happens only on Windows and Linux, Mac *is not* affected.
Flags: qe-verify+
Depends on: 1120735
P4 as polish, but we handle this inconsistently. NI mmaslaney to see if UX want to prioritize this higher.
Flags: needinfo?(mmaslaney)
Priority: -- → P4
Assignee: nobody → jaws
Attached patch PatchSplinter Review
Attachment #8581677 - Flags: review?(florian)
Status: NEW → ASSIGNED
Flags: needinfo?(mmaslaney)
Iteration: --- → 39.2 - 23 Mar
Points: --- → 2
Flags: firefox-backlog+
Blocks: 1132074
Comment on attachment 8581677 [details] [diff] [review]
Patch

I was surprised by this patch as I expected something that would make the focus ring invisible when the focus is from the click and the user hasn't used keyboard navigation, but Jared explained to me on IRC that this patch fixes the sizing of the highlight and that after this it doesn't look bad anymore. I'll buy that, but I would like if the bug title/commit message could be tweaked a bit so that there's no possible confusion about what to expect from this patch.
Attachment #8581677 - Flags: review?(florian) → review+
Summary: Clicking a button from the controls sidebar places a selection rectangle on it → The focusring for the controls in reader mode doesn't match the visible borders of the buttons
Iteration: 39.2 - 23 Mar → 39.3 - 30 Mar
https://hg.mozilla.org/mozilla-central/rev/d852fc6b7059
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
QA Contact: andrei.vaida
It was my understanding that this patch should fix only the focus-ring placed on the main buttons from Reader View's controls bar. 

The focus-rings now have the same size as the buttons themselves, but the other buttons from - say, "Type controls" [Aa], are still showing the same issue: http://i.imgur.com/hqAT0fd.png. Jared, are we planning to fix this in a separate bug?
Flags: needinfo?(jaws)
(In reply to Andrei Vaida, QA [:avaida] from comment #8)
> The focus-rings now have the same size as the buttons themselves, but the
> other buttons from - say, "Type controls" [Aa], are still showing the same
> issue: http://i.imgur.com/hqAT0fd.png. Jared, are we planning to fix this in
> a separate bug?

This issue seems to be isolated to Linux (Ubuntu 14.04 x64). I'm marking this fix as verified on Aurora 39.0a2 (2015-04-02), since Mac OS X 10.9.5 and Windows 7 (x64) are no longer affected, but I'll leave the ni? in place until we decide if the Linux-issue requires a separate bug or not.
Status: RESOLVED → VERIFIED
Yeah, we can file a separate issue for the focus rings in the popup on Linux. Thanks for verifying!
Flags: needinfo?(jaws)
Comment on attachment 8581677 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: reader view
[User impact if declined]: focus ring for reader view controls doesn't look right
[Describe test coverage new/current, TreeHerder]: landed on nightly
[Risks and why]: low-risk, small CSS change
[String/UUID change made/needed]: none
Attachment #8581677 - Flags: approval-mozilla-beta?
Attachment #8581677 - Flags: approval-mozilla-aurora?
Comment on attachment 8581677 [details] [diff] [review]
Patch

39 (aurora currently) already has this fix.
Attachment #8581677 - Flags: approval-mozilla-beta?
Attachment #8581677 - Flags: approval-mozilla-beta+
Attachment #8581677 - Flags: approval-mozilla-aurora?
Attachment #8581677 - Flags: approval-mozilla-aurora-
Depends on: 1151415
Verified fixed on Beta 38.0b2-build1 (20150406174117) as well, using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

Please note that Ubuntu 14.04 (x64) is currently affected by Bug 1151415.
Mistakenly filed against Firefox 38 and should be instead 38 Branch. Sorry for the spam. dkl
Version: Firefox 38 → 38 Branch
You need to log in before you can comment on or make changes to this bug.