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

Status: VERIFIED FIXED

(Firefox :: General, defect, P4)

Description

[Andrei Vaida [:avaida]]

Attachment: screenshot from Windows 8.1 (x86)

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.

Comment 1

[Andrei Vaida [:avaida]]

Attachment: screenshot from Ubuntu 14.04 (x64)

Comment 2

[Justin Dolske [:Dolske]]

P4 as polish, but we handle this inconsistently. NI mmaslaney to see if UX want to prioritize this higher.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 4

[Florian Quèze [:florian]]

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.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/cd9dda8f4cc1

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/d852fc6b7059

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d852fc6b7059

Comment 8

[Andrei Vaida [:avaida]]

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?

Comment 9

[Andrei Vaida [:avaida]]

(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.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, we can file a separate issue for the focus rings in the popup on Linux. Thanks for verifying!

Comment 11

[:Margaret Leibovic]

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

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8581677 [details] [diff] [review]
Patch

39 (aurora currently) already has this fix.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fd67405ba1de

Comment 14

[Andrei Vaida [:avaida]]

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.

Comment 15

[David Lawrence [:dkl]]

Mistakenly filed against Firefox 38 and should be instead 38 Branch. Sorry for the spam. dkl