Closed Bug 1214293 Opened 4 years ago Closed 4 years ago

2 items look focused in 'Fonts' window on about:preferences#content

Categories

(Core :: XUL, defect, trivial)

defect
Not set
trivial

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected
firefox44 --- affected
firefox48 --- fixed
firefox-esr38 --- affected

People

(Reporter: arni2033, Assigned: enndeakin)

References

(Regressed 1 open bug)

Details

Attachments

(2 files)

STR:   (Win7_64, Nightly 44, 32bit, ID 20151012030612, new profile)
1. Open about:preferences#content
2. Click "Advanced..." button
3. Press "Tab" key

Result:       2 buttons look focused: "Fonts for:" and "Proportional:"
Expectations: Only one button should look focused
Oh, I figured this out. Regression range in comment 1 means nothing, because I tested it on e10s.

The issue is sometimes reproducible on e10s and sometimes is not. But it's always reproducible on non-e10s. Bug 1055973 exposed this issue: before that bug was fixed, subdialogs in preferences didn't display focus outline for the 1st item in subdialog ("Fonts for:")
Blocks: 1055973
Keywords: regression
Can't reproduce on 42 beta on OS X. Might be windows-specific?

The whole thing is weird. Pretty sure we show the outlines for the :focus style, which only one item at a time should ever have.
I can reproduce on Windows. When this happens, two items match -moz-focus-ring, which should never happen. Neil, any idea why this occurs?
Component: Preferences → XP Toolkit/Widgets: XUL
Flags: needinfo?(enndeakin)
Product: Firefox → Core
I found out that if I open a profile in Beta/ESR (old versions), then close browser, open the same profile in latest Nightly, then in old version again - I don't see this bug (weird_1), but if I restart the old version, it appears again (weird_2). So it's NOT always reproducible for non-e10s.

These Steps are 100% for all versions (starting from bug 1055973), and for both e10s and non-e10s:
1. Open about:preferences#content
2. Restart the browser   [make sure that when it restarts, it opens only 1 window and its active tab
                          is about:preferences#content, and there's no other loaded tabs]
3. Click "Advanced..." button
4. Press "Tab" key

Result:       2 buttons look focused (have ":-moz-focusring" pseudoclass)a
Expectations: Only one button should look focused
Attached file initfocusrings
The issue here is that the 'Fonts for' dropdown shouldn't have a focus ring to start with. This patch ensures that the mShowFocusRings field is initialized properly for child windows.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Attachment #8676813 - Flags: review?(bugs)
Comment on attachment 8676813 [details]
initfocusrings

Could we call the new method UpdateShowFocusRings() given that it is called several times.

Though, why do we need it also in SetReadyForFocus?

(Looks like focus ring handling isn't updated for child processes, but that is a different bug. Could you perhaps file it?)
Attachment #8676813 - Flags: review?(bugs) → review+
I don't do firefox development actually, but still:
1) There's nothing wrong with that button being focused when that window opens. Does this fix only remove focus outline from _focused_element_? That's not good for accessibility, I think.
2) Could you please don't land this patch for a while? I'm able to reproduce it on web page.
> testcase_url: https://dl.dropboxusercontent.com/s/zz4ivv2jw6wwl3k/del%20testcase%20-%20-moz-focusring%20bug.html?dl=0
Maybe there's a more general way to fix this for all possible cases? Should I file a separate blocking bug?
This patch only removes the focus outline, as focus outlines don't appear by default on Windows.

> testcase_url: https://dl.dropboxusercontent.com/s/zz4ivv2jw6wwl3k/del%20testcase%20-%20-moz-focusring%20bug.html?dl=0

I don't get mutliple focus rings in this test case.
> (Looks like focus ring handling isn't updated for child processes, but that
> is a different bug. Could you perhaps file it?)

The child process case (e10s) is bug 1174798.
(In reply to Neil Deakin from comment #9)
> I don't get mutliple focus rings in this test case.
Screenshot:
> http://ssmaker.ru/2368492d.png
Well, I mentioned that original bug isn't always reproducible... I noticed that the original bug is reproducible only when this newly found bug is reproducible and vice versa.

If you can't reproduce the second one - open about:preferences#content and try to reproduce the original bug - it won't happen. I hope after that you'll believe me.

I can only suggest you to follow STR from comment 5 (or similar)
1. Open about:preferences#content (Or New Tab. Maybe it works with any about: pages)
2. Restart the browser   [make sure that when it restarts, it opens only 1 window and its active tab
                          is about:preferences#content, and there's no other loaded tabs]
3. Open testcase:
> https://dl.dropboxusercontent.com/s/zz4ivv2jw6wwl3k/del%20testcase%20-%20-moz-focusring%20bug.html?dl=0
4. Press "Tab" key
I don't see any issues with the patch applied with any of these steps.
https://hg.mozilla.org/integration/mozilla-inbound/rev/90edb8c62dee69fe55faf84507d70570f9d8eaad
Bug 1214293, initialize show focus rings state properly in all child frames, r=smaug
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=16323984&repo=mozilla-inbound
Flags: needinfo?(enndeakin)
Has STR: --- → yes
https://hg.mozilla.org/integration/mozilla-inbound/rev/becc2eec160165d310830b65ae823973e2849865
Bug 1214293, initialize show focus rings state properly in all child frames, r=smaug
https://hg.mozilla.org/mozilla-central/rev/becc2eec1601
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160320030409
Status: RESOLVED → VERIFIED
Flags: needinfo?(enndeakin)
Depends on: 1263330
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.