about:config: no focus ring around enter button

RESOLVED FIXED in mozilla2.0b1

Status

()

Core
General
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: philippe (part-time), Assigned: Neil Deakin)

Tracking

({regression})

Trunk
mozilla2.0b1
x86
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

8 years ago
STR:
1. load 'about:config'
AR: "I'll be carefull, I promise" button doesn't show a focus ring
ER: the button shows a focus ring to indicate immediate action is possible,

works
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100421 Minefield/3.7a5pre
http://hg.mozilla.org/mozilla-central/rev/cdc8bf25220e

fails
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100422 Minefield/3.7a5pre
http://hg.mozilla.org/mozilla-central/rev/ab4c6ba5ff70

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cdc8bf25220e&tochange=ab4c6ba5ff70

--> bug 418521 is the likely suspect
(Reporter)

Comment 1

8 years ago
Narrower regression range:
works: /rev/e7e7a4fe417c
fails: /rev/f3abfe490d7f
--> bug 418521
Blocks: 418521
(Assignee)

Comment 2

8 years ago
This is correct behaviour on Mac. Focus rings only show on textboxes/lists and when the tab key has been used to focus.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → INVALID
(Reporter)

Comment 3

8 years ago
In which case, the button ought to be pulsating blue/graphite to indicate it is the first responder.

Do you want a new bug for that or do we reopen this one ?
(Assignee)

Comment 4

8 years ago
(In reply to comment #3)
> In which case, the button ought to be pulsating blue/graphite to indicate it is
> the first responder.

I don't think it is though. The pulsating would imply that it was the default button and would activate when pressing Enter which is not the case.
(Reporter)

Comment 5

8 years ago
(In reply to comment #4)

Right, but pressing the spacebar activates the button (in which case it would need a focussing ring). And it is the focussed element in the tab chain.
(Assignee)

Comment 6

8 years ago
Created attachment 441392 [details] [diff] [review]
basic patch

OK, can you try this patch? I think this will fix a couple of issues on Mac:.

Tryserver builds will appear at https://build.mozilla.org/tryserver-builds/neil@mozilla.com-allfocusrings/
Assignee: nobody → enndeakin
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(Reporter)

Comment 7

8 years ago
(In reply to comment #6)

> Tryserver builds will appear at
> https://build.mozilla.org/tryserver-builds/neil@mozilla.com-allfocusrings/

At least the issue in this bug is fixed. ty.
(Assignee)

Comment 8

8 years ago
Created attachment 442104 [details] [diff] [review]
fix focus rings on mac

This patch:
 - makes focus rings appear by default on Mac. Note that elements do not get focused when clicked on Mac.
 - this also fixes the test failure in bug 560898
 - makes the selector accurate for textboxes/lists, before the :-moz-focusring property wouldn't apply for these on Mac

Not sure who a good reviewer is here.
Attachment #441392 - Attachment is obsolete: true
Attachment #442104 - Flags: review?(jonas)
Comment on attachment 442104 [details] [diff] [review]
fix focus rings on mac

I really have no idea what's going on here, so I don't think I'm the right reviewer :(
Attachment #442104 - Flags: review?(jonas)
I think the dom/base/nsGlobalWindow.cpp part of the patch is correct, but I'm not a qualified reviewer. I haven't looked at the test and I don't understand how the patch changes selector matching for textboxes/lists. Did you just miss including that part in the patch?
(Assignee)

Comment 11

8 years ago
Without this patch, the :-moz-focusring property wouldn't apply to textboxes and lists even though focus rings appear to be drawn for them because the :focus pseudo is used instead. In fact, on Mac, focus rings are always drawn for all focused elements, whether textboxes or lists, so with this patch the :-moz-focusring happens to apply to them correctly.
Ah, I see.
(Assignee)

Comment 13

8 years ago
Comment on attachment 442104 [details] [diff] [review]
fix focus rings on mac

I think instead I'm going to fix up html elements as well.
Attachment #442104 - Attachment is obsolete: true
(Assignee)

Comment 14

8 years ago
That patch which also fixes the bug is 564277
>I don't think it is though. The pulsating would imply that it was the default
>button and would activate when pressing Enter which is not the case.

This is a kind of complex case, because we are basically dealing with an in content dialog box.  I think the correct behavior is to mirror how this would behave if it were a real dialog box, and give the button default styling + activate it on enter.  (either way it can't be the case that hitting space activates the button but hitting enter does not).  Another instance of this case is the in content session restore, which also acts as a sort of tab modal dialog box.
Looks like this was fixed by bug 564277.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3b1
You need to log in before you can comment on or make changes to this bug.