418521 - Focus ring appears on mouse interactions (as opposed to only when tabbing through items)

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Examples of the focus ring appearing on mouse interactions

The focus ring currently appears on any widget, hyperlink or image that has the focus.  Ideally, the focus ring should only be visible if the user has started to tab through elements, providing feedback on which widget has the focus.

This is (in my opinion) one of the largest polish problems we currently have with XUL.  Web developers also occasionally complain that the focus ring in the content area detracts from the beauty of their carefully crafted designs.

Examples provided in the attachment, my apologies if this is a dupe.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Probably no chance of fixing this in 1.9, though Cc'ing roc to see if he has any quick fix ideas.  I'm pretty sure this isn't just a CSS change, sadly.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

we'd probably need a new CSS pseudo-selector to indicate elements that have been keyboard-focused. Definitely not happening for 1.9.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

Aaron: do you know how feasible fixing this would be?

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

This is a pretty big polish problem, especially on Windows. No other application or browser does this, and while I'm a big fan of drawing the focus rings when one starts tabbing, I don't think they need to be drawn on every click. Ends up looking quite messy.

Comment 5

[Jesse Ruderman]

Do other browsers apply :focus styles for mouse-clicked items?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I hear you Mike, but we've always done this, so it's just one more "nice to have" thing for 1.9 and "wanted" status is completely appropriate.

Comment 7

[Alex Faaborg [:faaborg] (Firefox UX)]

>Do other browsers apply :focus styles for mouse-clicked items?

Safari turns links red like we do, IE7 has no change on mouse click.  I'm in
favor of a visual affordance for down click (although I think red is a bit
much), but the focus ring looks really unpolished, especially when clicking on
images.

Comment 8

[Jesse Ruderman]

I find the focus outline useful for answering the question "which link did I last click?" in bug lists.

Comment 9

[Aaron Leventhal]

FWIW focus changes have a history of causing nasty regressions (not just from me) :)

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fixing this doesn't really require focus changes.

Comment 11

[Aaron Leventhal]

I'm afraid of this breaking a11y of Javascript widgets. A lot of effort has been put into making Dojo's JS widgets accessible. For example, when you click on an item in a group of items it may be using a :focus rule to show the expected indication that the individual item is the active one.

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Example of how the focus ring exposes the implementation model

In this screen shot we see how the designer of getmiro.com crafted a beautiful download button, but the focus ring exposes the actual region of the image to the user.  This is an example of the implementation model of the page (the region of the actual image) being incorrectly exposed at the interface level to the user.

Comment 13

[Kai Liu]

It should be noted that, in Windows, this is a UI preference.  Uncheck the hide-keyboard-navigation option (which is checked by default) in the advanced display prefs in XP and then fire up Internet Explorer.  You'll see that, like Firefox, IE will apply a focus ring to everything that gets clicked by the mouse.

Being able to visually verify and check exactly what you clicked on is important for accessibility and usability (especially for images and for links that have been styled to not change color), so if we were to stop this behavior, we must give the user some way to re-instate it, either by honoring the appropriate preferences in Windows or through a Firefox preference (could be hidden) for non-Windows systems that don't have such a global setting

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

One option would be to have a -moz-keyboard-focus pseudoclass that is set for the focused element when the focus was set by keyboard, and continue allowing :focus to match any focused element, but change our UA style sheet so that most of our elements use -moz-keyboard-focus. Shouldn't be hard and I don't see that that would break anything. Just need someone to step up.

Comment 15

[Markus Stange [:mstange]]

That sounds like a nice solution for content, but at least in XUL chrome we'd end up with a behaviour that's different from all platforms I've tested on.

Here are the results of my tests:

- Mac OS X chrome:

  On Mac OS X, a focus ring is shown all the time. When opening a new window,
  it's usually on the first focusable element.
  Clicking on a control doesn't change the focus ring's position. Say the focus
  is on the first checkbox in a window. After a click on the fifth checkbox,
  the focus ring is still on the first checkbox, and pressing Tab moves focus
  to the second checkbox.
  There are at least two exceptions to this: Textboxes and lists. Both controls
  grab focus on click because they expect keyboard input - in lists, you can
  change the selection using the arrew keys.
  In chrome, clicking on plain, unfocusable area doesn't reset focus.

- Safari content area:

  In Safari, only clicks on textboxes and <select>s focus the clicked element.
  All other clicks reset the focus to "nowhere"; pressing Tab after such a
  click focuses the first focusable element on the page.

- Windows XP chrome:

  (These tests might be flawed - the preference Kai mentioned in comment 13
  didn't have any effect for me; I might be doing something wrong.)

  Windows XP has a completely different focus model. On Windows, every click
  sets the focus on the clicked control, just like Firefox is doing now.
  However, no focus ring is drawn as long as you haven't hit a keyboard
  navigation key.
  As soon as you hit the tab or arrow keys, a per-window switch is flipped:
  from now on, focused elements in this window will always draw a focus ring,
  even those that have been focused by a mouse click. (However, it looks like
  you can reset the switch when changing to a different tabpanel...)
  It's not possible to reset focus to "nowhere".

- Internet Explorer content area:

  Internet Explorer 6 and 7 behave the same as in chrome - focus follows
  mouse but the focus ring isn't drawn until the keyboard navigation switch is
  flipped.
  The Internet Explorer 8 Beta supports the :focus pseudoclass and has adopted
  Firefox's behaviour - for websites that are in IE 8 mode, the focus ring is
  always drawn, even without "flipping the switch".
  (At least it looks like that's the intended behaviour; sometimes it doesn't
  really work, some focus rings are cut off etc... it's apparently still a
  little buggy.)
  There's another interesting thing: Clicking anywhere on the page doesn't
  reset focus to "nowhere" - instead, the element under the mouse pointer
  (div, p or whatever) is focused (without focus ring), so you can start
  tabbing near the click position.

I haven't tested on linux yet.

I'm still thinking about a model that would work for Gecko, feel native on both platforms, be consistent for web content and not too dangerous to implement.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #15)
> - Mac OS X chrome:
> 
>   On Mac OS X, a focus ring is shown all the time. When opening a new window,
>   it's usually on the first focusable element.
>   Clicking on a control doesn't change the focus ring's position. Say the focus
>   is on the first checkbox in a window. After a click on the fifth checkbox,
>   the focus ring is still on the first checkbox, and pressing Tab moves focus
>   to the second checkbox.

What are you testing? The native Mac apps I've tried don't ever give focus to checkboxes (on Leopard anyway).

I didn't notice that until just now, though, which suggests maybe it isn't very important.

Comment 17

[Markus Stange [:mstange]]

> What are you testing? The native Mac apps I've tried don't ever give focus to
> checkboxes (on Leopard anyway).

Ah, I forgot to say that. I've turned on "full keyboard access" in the Keyboard Shortcuts section of the Keyboard preferences. That makes these controls focusable by keyboard.

Comment 18

[Alex Faaborg [:faaborg] (Firefox UX)]

Note bug 437296 for potentially changing the behavior on OS X

Comment 19

[Martijn Wargers (dead)]

See also discussion in bug 262578.

Comment 20

[Alex Faaborg [:faaborg] (Firefox UX)]

this bug is eligible for bug 462080 :)

Comment 21

[[:Aleksej]]

An example of how the focus outline can be useful:
* Go to a page which links to many pages, some or all of which are visited.
* Middle‐click some of the links in order.
* Check the opened tabs, closing some or all of them.
* Go back to the original tab and continue clicking from the place where you have stopped (it is marked by the focus outline).

Comment 22

[Jesse Ruderman]

I rely on focus outlines in the same way Aleksej describes.

Comment 23

[Alex Faaborg [:faaborg] (Firefox UX)]

At the very least we shouldn't have this issue in chrome.  In terms of content, I agree that the focus ring can be useful on hyperlinks (in addition to changing their color), although we might want to do something like more visually appealing than the current dotted line.

Comment 24

[Nochum Sossonko [:Natch]]

1. any website can disable this focusing with the following simple css || a:focus {outline:none;} ||
2. shouldn't website authors be responsible for visual feedback?
3. how about elements with onclick handlers?
4. IMHO coloring the link red shoulb be more than enough visual feedback...
5. if that's not convincing enough, how about just an underline or something?

Comment 27

[Boris Zbarsky [:bzbarsky]]

What about website controls focused via focus() calls?  Should those be treated as "clicked on" or as "tabbed to" for visual feedback?

I often find that websites will focus some random textfield when I load the site, and the blinking 1-px-wide caret is not very obvious... the focus ring is, though.

Comment 28

[Aaron Leventhal]

(In reply to comment #27)
> What about website controls focused via focus() calls?  Should those be treated
> as "clicked on" or as "tabbed to" for visual feedback?

.focus() needs to provide visual feedback, because authors often have to implement their own keyboard navigation for custom JS widgets like tree views or spreadsheets, etc. They capture a key and focus the next item, when appropriate. So, unless you are going to try and determine why the focus moved for a .focus(), I would just visually show all of those.

Here's an example:
http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/test_Tree.html
(Tab into the tree and arrow around).

Comment 29

[Neil Deakin]

Attachment: possible patch

This patch implements a pseudoclass which can be used for specific focus methods. It applies on top of the focus work patch in bug 178324.

element:-moz-focused-by(mouse) { ... }
element:-moz-focused-by(key) { ... }
element:-moz-focused-by(application) { ... }

A second possible implementation is to instead use:

element:-moz-focus-ring { ... }

which would apply whenever the element was focused and a focusring should appear, which would be determined internally. This would offer less specific control, but would be easier to just use one rule. It would also easily fix bug 377320.

Thoughts?

Comment 30

[Dão Gottwald [:dao]]

This tells me that we need :-moz-focus-ring rather than :-moz-focused-by(...):

(In reply to comment #13)
> It should be noted that, in Windows, this is a UI preference.  Uncheck the
> hide-keyboard-navigation option (which is checked by default) in the advanced
> display prefs in XP and then fire up Internet Explorer.

Comment 31

[Neil Deakin]

(In reply to comment #30)
> This tells me that we need :-moz-focus-ring rather than :-moz-focused-by(...):
> 
> (In reply to comment #13)
> > It should be noted that, in Windows, this is a UI preference.  Uncheck the
> > hide-keyboard-navigation option (which is checked by default) in the advanced
> > display prefs in XP and then fire up Internet Explorer.

OK, support for the preference is added by bug 25894. I think I'll put together a patch that does :-moz-focus-ring without this preference just to try it out and determine which approach is actually needed.

Comment 32

[Neil Deakin]

Attachment: -moz-focusring

This patch implements -moz-focusring which matches when a focus ring should appear. For testing, it matches only when the keyboard is used on all platforms. Really, it would use the setting on Windows as needed.

Comment 33

[Dão Gottwald [:dao]]

Much like bug 25894, it's slightly more complex than just a binary pref; see comment 15. The section on OS X is also interesting.

Comment 34

[Neil Deakin]

(In reply to comment #33)
> Much like bug 25894, it's slightly more complex than just a binary pref; see
> comment 15. The section on OS X is also interesting.

The behaviour described there makes it sound more complicated that it actually is. This is the actual behaviour:

1. On both Mac and Windows, focus rings do not appear within a window until the Tab key is pressed, at which point they always appear whatever the focus method. On Windows, the Alt key also does this, and also makes accesskeys appear as well.

2. The Windows setting (in the Display control panel -> Effects button) may be used to toggle the behaviour to always showing focus rings, even when the Tab or Alt key is not pressed. This is the behaviour that my Linux machine exhibits by default.

3. On Windows, clicking an element always focuses it. On Mac, clicking an element never focuses it.

4. Textboxes and lists are special on Mac. They may be focused by clicking them and focus rings are always drawn. For this, we should be able to keep the existing focusring styles.

The Mac behaviour for point 3 is not currently implemented and is not this bug (or at least, is a different feature/patch that isn't focusring related)

Comment 35

[Dão Gottwald [:dao]]

But without 3, it seems like we'd introduce new non-native behavior on OS X: omitting the focus ring while actually changing focus.

Comment 36

[Neil Deakin]

Yes, we should implement that first. I had actually intended to make the -moz-focusring match check windows only and would always match on other platforms until then. But I didn't for now for testing purposes.

Comment 37

[Dão Gottwald [:dao]]

(In reply to comment #36)
> I had actually intended to make the
> -moz-focusring match check windows only and would always match on other
> platforms until then.

OK, that seems correct, except for "until then". If focus wouldn't be changed in the first place on OS X, :-moz-focusring would still always match in accordance with :focus.

Comment 39

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: quick vanilla demo of mouse vs keyboard focus indication

Sorry I'm coming in late so not sure if this is just noise, it has been a long day and I might have missed a comment or two, but this might be a way to go. No points for style here ;)

Comment 40

[alexander :surkov (:asurkov)]

I would say ugly focus ring is not a point to not show focused element. I
believe we must indicate somehow focused element because otherwise it's not
possible to say where is the focus and what tab key is supposed to do. I think
this way is definitely keyboard friendly. I think we should replace ugly focus
ring by more gladden-the-eye way. On another hand it's worth to make Firefox
behaving like platform applications do, otherwise it might annoy users. In this
light I would say it's worth to have a preference which points if OS rules or
keyboard friendly rules are used.

Comment 41

[David Bolter [:davidb] (NeedInfo me for attention)]

Yes. I agree the right way to go is to always indicate where focus is.

Comment 42

[Alex Faaborg [:faaborg] (Firefox UX)]

>it's worth to make Firefox
>behaving like platform applications do, otherwise it might annoy users. In this
>light I would say it's worth to have a preference

I'm fine with an about:config pref, but by default I would like us to match the behavior of all other operating systems and browsers.

Comment 43

[alexander :surkov (:asurkov)]

(In reply to comment #42)
> >it's worth to make Firefox
> >behaving like platform applications do, otherwise it might annoy users. In this
> >light I would say it's worth to have a preference
> 
> I'm fine with an about:config pref, but by default I would like us to match the
> behavior of all other operating systems and browsers.

This makes sense I think we should try to correspond to native browser on OS where firefox is used by default. One thing I want is this work should be performed the same time so we won't get usability regressions.

Comment 44

[Not interested in Mozilla any more]

Please resent commenters you are completely missing the point of this bug. The point stated by the reported, and which I agree with, is that the focus SHOULD NOT BE SHOWN if you simply click a link on a page.

Take an example of a frames setup with a navbar in the left hand frame and a document window in the right. When you click a link in the left you get the document appearing in the right. The issue is that clicking that link in the left causes a focus ring to appear (and stay there) in the left panel. Given the user has probably switched their attention to the right hand frame this focus ring is wrong. Please take account of this issue and the OP when thinking about this bug.

Comment 45

[Neil Deakin]

Attachment: work in progress

This patch:
 - makes XUL elements (except lists/textboxes) not focus on mouse clicks on Mac. I haven't changed HTML elements in this patch.
 - focus rings only appear after a key is pressed to focus an element on all platforms (should be Windows only but not for this testing)
 - adds a :-moz-focusring pseudoclass for checking if focus rings should appear

Only tested on Mac.

Comment 46

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P0 - There can be only one bug designated P0 at a time, this is the single highest priority active polish bug in existence.

This is basically a polish bug on nearly every single interaction the user has with chrome, the absolutely highest priority polish bug we have right now.

Comment 47

[Markus Stange [:mstange]]

Neil, this patch is fantastic!

Some comments:
 - The downloads richlistbox doesn't grab focus on click. It should.
 - layout/style/ua.css should probably use :-moz-focusring instead of :focus?
 - Radio buttons in radio groups, e.g. the buttons in the preferences window,
   still get [focused="true"] on mousedown, but I suppose fixing that isn't
   part of this bug.

I think it's perfectly fine to make the Mac focus model of "don't focus on mousedown" XUL-only.
How do you feel about using the Windows focusring model for HTML content on all platforms? That way, content authors would have a consistent cross-platform focus model.

Comment 48

[Neil Deakin]

(In reply to comment #47)
> I think it's perfectly fine to make the Mac focus model of "don't focus on
> mousedown" XUL-only.
> How do you feel about using the Windows focusring model for HTML content on all
> platforms? That way, content authors would have a consistent cross-platform
> focus model.

I think that would be a question best asked of Alex.

The Windows model is:
- click an element to focus it
- focus rings only appear once Tab has been pressed once for that window/dialog. After that, they always appear whatever the focus method (mouse, key, etc).
- the system setting may be changed to enable focus rings always, as well as accesskey underlines.

An open question is whether the 'Tab has been pressed once for that window/dialog' applies per top-level window, per frame, per tab or per document.

Note also that this bug is actually dependent on 25894. I don't think that we should be implementing this without honouring the system setting.

Comment 49

[Daniel Luz]

(In reply to comment #48)

I guess he was just referring to the "click always focuses control" rule, not the whole invisible focus ring intricacies brought by Windows 2000.

Now, regarding this idea, I'm not sure I like it, as it wouldn't solve the most visible aspect of the user experience (focus rings floating around when clicking links), unless common hyperlinks (those which do not set tabindex) don't get focused.

Analyzing other browsers, it appears WebKit (Safari 4 and Chrome for Mac preview) does use click-always-focuses behavior for tabindex="0" elements, though with a very specific exception for <a>, whether it's actually a link or not. This last part seems slightly flawed to me.

Comment 50

[Markus Stange [:mstange]]

(In reply to comment #49)
> (In reply to comment #48)
> 
> I guess he was just referring to the "click always focuses control" rule, not
> the whole invisible focus ring intricacies brought by Windows 2000.

No, I was indeed referring to the "invisible focus ring intricacies brought by Windows 2000".

Comment 51

[Alex Faaborg [:faaborg] (Firefox UX)]

> That way, content authors would have a consistent cross-platform
> focus model.

I think everyone will likely agree that establishing a consistent cross platform focus model for the Web has a lot of advantages, so the question becomes what attributes should that focus model have.

Note that this entire comment is just about the idea of changes to the focus model in the content area (chrome should definitely be platform specific, and match the correct behavior on each platform).

>I think that would be a question best asked of Alex.
>
>The Windows model is:
>- click an element to focus it
>- focus rings only appear once Tab has been pressed once for that
>window/dialog. After that, they always appear whatever the focus method (mouse,
>key, etc).
>- the system setting may be changed to enable focus rings always, as well as
>accesskey underlines.

So think the main points in favor of this approach is:

-Cleaner visual appearance of Web sites (Web developers currently complain that we draw focus rings all over their carefully designed image maps)

-When you tab on OS X, the behavior is different, but we want to violate the platform conventions anyway (details in bug 437296)

Currently the main points against this approach that come to mind are:

-It is sometimes useful to see the focus on hyperlinks, so that you can tell which hyperlink you recently clicked on, and for instance use that information to click on the next hyperlink in a list.

-Drawing the focus on text fields is kind of useful, because it gives you an additional indication beyond the blinking cursor where text is going to show up when you start typing.  Currently Chrome on Windows uses a bright orange highlight on text fields, and aside from being completely non-native, it is actually kind of nice.

So just to throw an idea out there, what if we created a hybrid approach for the Web's focus model on all platforms that had these particular properties:

1) When you start to tab, every control will display visible focus, including things like combo boxes, check marks, and buttons.  This behavior is not standard on OS X, but is nonetheless useful.

2) When you are using the mouse, some controls do actually display visible focus, including text fields and hyperlinks (but not combo boxes, check boxes, radio buttons, buttons, etc.)  This is not standard on Windows, but is nonetheless useful.

3) The focus appearance is a bit fancier than a single dotted line, and can be styled per platform.  So even though the behavior may not be native (depending on the specific control and specific platform), the visual presentation can be customized so that it still integrates well with the surrounding OS, like using the correct color palette, etc.

4) (crazy talk) the focus visual appearance can be customized by the Web author.  I have no idea how much standards work we would need to do ahead of time on this.

Comment 52

[Alex Faaborg [:faaborg] (Firefox UX)]

Another thing that comes to mind is that I believe Vista is using a different focus model than XP, although I can't confirm since I'm not in the office and only have XP running.  I think on Vista text fields get a different styling when they have the focus (similar to OS X), however this could just be an IE in the content area behavior, I know there are using a different widget styling there (for instance combo boxes look more like text fields as opposed to buttons).

Either way, I'm proposing that even on XP, a platform that has never drawn text fields with a visual focus, we consider drawing text fields with a visual focus (using a color out of the Luna color palette).

Also, since Linux users may quickly comment that I am leaving them out, I honestly don't know anything about the gnome focus model (including things like if it changes based on the OS theme), so any insight you anyone can provide would be really appreciated :)

Comment 53

[Alex Faaborg [:faaborg] (Firefox UX)]

Does drawing the focus on a hyperlink only when the user goes away from a page and then returns to it (either through a tab switch or a back/forward navigation) a reasonable idea?  You don't really need the focus the very moment you are clicking on the link with the mouse, since it is pretty obvious what you are clicking on given the mouse cursor position.

Comment 54

[(no longer active)]

(In reply to comment #53)
> Does drawing the focus on a hyperlink only when the user goes away from a page
> and then returns to it (either through a tab switch or a back/forward
> navigation) a reasonable idea?  You don't really need the focus the very moment
> you are clicking on the link with the mouse, since it is pretty obvious what
> you are clicking on given the mouse cursor position.

Yes, I think so, because otherwise it would defeat the following goal:

-Cleaner visual appearance of Web sites (Web developers currently complain that
we draw focus rings all over their carefully designed image maps)

Comment 55

[Alex Faaborg [:faaborg] (Firefox UX)]

yeah, and it would allow us to make the focus more obvious, which is useful since you are returning to the page and are actively looking to see what hyperlink had (I guess now has) the focus.

Comment 56

[Neil Deakin]

The Linux behaviour is what is currently implemented. Clicking focuses items, and they always show focus indicators.

> 3) The focus appearance is a bit fancier than a single dotted line, and can be
> styled per platform.

The appearance of focus rings is just specified in the user agent stylesheet (ua.css/html.css/etc) via:

element:focus { ... }

and can be overriden by pages.

Note that the :focus pseudoclass applies whenever the element is focused. We would either want to use the new pseudoclass (:-moz-focusring) or change :focus to only apply when focus rings should be drawn. The former seems better.

Also note that the focus indicator on various elements is drawn by native theme code.

Finally, note that in the patch I have changed the behaviour for XUL and not HTML, and do not distinguish between content and chrome. The former is easy to distinguish in places where focus appearance is handled, but I don't think it's particularly feasible to determine whether in a content or chrome window.

Comment 57

[Neil Deakin]

So this is the information I need to finish work on this bug:

Which sets of elements fit within which of the following categories:
 A. Clicking the element focuses it and shows a visible focus ring.
 B. Clicking the element focuses it but does not give a visible indicator.
 C. Clicking the element does not focus it.

I propose the following:

When clicking on XUL elements:
  On Mac, textboxes and lists: A
  On Mac, all other elements: C
  On Windows, when tab or an accesskey has already been pressed in that window: A
  On Windows, with the 'hide underlined letters for keyboard navigation' system setting on: B
  On Windows, with the 'hide underlined letters for keyboard navigation' system setting off: A
  On Linux, all XUL elements: A
When clicking on HTML links, on all platforms: B, but when returning to the page, show the focus indicator.
When clicking on non-link HTML elements, on Windows, with the 'hide underlined letters for keyboard navigation' system setting off: A
When clicking on HTML inputs, on all platforms: A
When clicking on non-link and non-input HTML elements, on all platforms: B
When focusing an element with tab or an accesskey, or by calling the element's focus() method: A

The :focus pseudoclass may be used by authors to set the appearance when the element is focused (This would draw in cases A and B) . The :-moz-focusring pseudoclass may be used to set the appearance only when the element should draw a focus ring (case A only).

Note that the 'Hide underlined letters for keyboard navigation' defaults to on and affects both underline accesskey indicators and focus ring display.

Comment 58

[Markus Stange [:mstange]]

Your proposal sounds perfect to me.

Comment 59

[Dão Gottwald [:dao]]

(In reply to comment #57)
> When clicking on HTML links, on all platforms: B, but when returning to the
> page, show the focus indicator.

Keep in mind that links can be opened in new tabs/windows, so "returning to a page" would have to include switching tabs/windows.

Comment 60

[Neil Deakin]

Attachment: implement as descrbed in comment 57

Builds are at https://build.mozilla.org/tryserver-builds/[e-mail]-focuscombined/

Comment 61

[Neil Deakin]

Should also mention that the patch from bug 504367 should be applied first as it fixes some problems with focus and back/forward navigation.

Comment 62

[Neil Deakin]

Two things I didn't change:
 - the appearance of input fields. They will just look as they do now. The specific appearance I think can be done in a followup bug.
 - the focus indicator on links appears when switching back to the tab, or by navigating back in history. The excepions are when the page isn't in the bfcache as there isn't anything to restore anymore, and also, it doesn't get shown when a link opens a new window and the original window is switched back to.

Comment 63

[Emil Ivanov]

(In reply to comment #62)
>  - the focus indicator on links appears when switching back to the tab, or by
> navigating back in history. The excepions are when the page isn't in the
> bfcache as there isn't anything to restore anymore, and also, it doesn't get
> shown when a link opens a new window and the original window is switched back
> to.

Isnt it better to show outline when alt is pressed. That way the outline will be available only when is requestesd by the user.

No keyboard - no outline.

Hiding the outline when mouse is used and then showing it in some cases will be almost the same situation like is now, but will be seen not so often.

Comment 64

[Markus Stange [:mstange]]

diff --git a/layout/style/forms.css b/layout/style/forms.css
-input[type="image"]:focus {
+input[type="image"]:moz-focusring {

You forgot a dash here.

Comment 65

[Neil Deakin]

(In reply to comment #63)
> Isnt it better to show outline when alt is pressed. That way the outline will
> be available only when is requestesd by the user.

I wasn't aware that happened, but I'll add back code to do that.

Comment 66

[Emil Ivanov]

(In reply to comment #62)
> The excepions are when the page isn't in the
> bfcache as there isn't anything to restore anymore, and also, it doesn't get
> shown when a link opens a new window and the original window is switched back
> to.

BTW maybe this is possible too although im not sure.

The outline could be there all the time just change the color from transparent to what is required. Visually the outline will be invisible but it will be on the exact position when needed.
So when alt is pressed or tab, the focus will appear on place.

Comment 67

[Boris Zbarsky [:bzbarsky]]

> You forgot a dash here.

Please also add an automated test that would catch that.

Comment 68

[Alex Faaborg [:faaborg] (Firefox UX)]

I'll try to get to this ui-review soon, overall behavior sound great but there are a lot of little details to check are native behavior on each platform.

Does the current patch only effect XUL?  Some of the ideas discussed around Comment #51 should probably be spun into a separate bug since there are a lot of implications to discuss.

Comment 69

[Neil Deakin]

(In reply to comment #68)

> Does the current patch only effect XUL?

It affects HTML as well. The patch implements the behaviour described in comment 57, with the caveats mentioned in comment 62.

Comment 70

[Alex Faaborg [:faaborg] (Firefox UX)]

Sorry about the delay with the ui-review, still catching up on things now that I'm back in the office.

Comment 71

[Mike Beltzner [:beltzner, not reading bugmail]]

Can we not just get a ui-review on the actual changes as described in comment 57 (if that's what's implemented) and then we can follow up with issues as they're discovered? Or is the issue that you're checking to make sure that what's described in comment 57 is considered native on various platforms?

Comment 72

[Alex Faaborg [:faaborg] (Firefox UX)]

I hit a bunch of errors trying to apply the patch, could you check if it might be bitrotten, or post links to new try server builds?

>Can we not just get a ui-review on the actual changes as described in comment
>57 (if that's what's implemented)

The changes sounds right, but I was planning on trying them all out on all platforms for the review.  Otherwise I can just approve the described changes.

Comment 73

[Neil Deakin]

Attachment: updated patch

Comment 74

[Neil Deakin]

Builds are at https://build.mozilla.org/tryserver-builds/[e-mail]-focusrings2/

Comment 75

[Daniel Luz]

So, any news on this bug?

I've been trying the OS X build for the last few weeks, and it seems to work fine -- in fact, it's much better than I had expected, particularly the content behavior (you'll see on comment #49 that I originally thought this approach wouldn't be very good). I just have two quirks to report:

- When changing tabs (or any tab-like widget), focus should never stay on an element from the previous tab. Open the preferences window on the General tab, focus the "Home Page" edit box, switch to another tab (say, Advanced) using the mouse, and press the down arrow. A URL suggestion drop-down will appear, showing the URL box from the General tab is still focused.

- If arrow keys are somehow used to move between elements (basically a chrome window where the focus starts on a checkbox, radio button or button), the focus ring should appear, just as if you used the Tab key. A practical example is the proxy settings window: if you open it and press up/down, the radio button selection will move, but nothing will indicate where your focus actually is. Related to this particular quirk:

    * I'm pretty sure arrow keys under these circumstances also trigger
      the "keyboard mode" on Windows, but I can't test them right now.

    * Maybe we could just always show the focus ring on chrome windows.
      An example where such behavior is clear is the preferences window
      for Safari -- there's always a focus ring somewhere. Windows in
      other apps that don't show a focus ring anywhere seem to actually
      have no focused control (i.e., pressing the spacebar has no
      effect), unlike the behavior in these builds, where we're hiding
      the fact that there's a control with focus.

I was going to try a Linux build now, but the builds seem to be gone.

Comment 76

[Markus Stange [:mstange]]

@@ -549,16 +557,22 @@ nsXULElement::IsFocusable(PRInt32 *aTabI

+#ifdef XP_MACOSX
+  // on Mac, mouse interactions only focus the element if it's a list
+  if (aWithMouse && IsNonList(mNodeInfo))
+    return NS_OK;

This should be return PR_FALSE.

Comment 77

[Alex Faaborg [:faaborg] (Firefox UX)]

Really sorry about the lag on the ui-review.  I'm now finally back in the office and I can get you a review if you post new tryserver builds, since I want to check this out on all platforms.

Comment 78

[Neil Deakin]

Alex, there should be try server builds with the id 'bug418521-focusrings' coming in the next hour or so.

Comment 79

[Alex Faaborg [:faaborg] (Firefox UX)]

>Alex, there should be try server builds with the id 'bug418521-focusrings'
>coming in the next hour or so.

Hm, not seeing a windows build: https://build.mozilla.org/tryserver-builds/[e-mail]-bug418521-focusrings/

Comment 80

[Neil Deakin]

A very recent change that caused a build error. A Windows build is available at https://build.mozilla.org/tryserver-builds/[e-mail]-bug418521-focusrings2/

Comment 81

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 394820 [details]
updated patch

Overall I'm going to give this a ui-r+ since the behavior is a lot better than what we currently have.

I found what I think are 7 possible errors, if it is easy to correct some of these we can consider them nits, otherwise we'll just deal with them in follow up bugs.  This spreadsheet lists what I think the right behavior is, and where the current patch deviates: http://spreadsheets.google.com/pub?key=t8sBdbRxc_OGAZavmb-5F9g&output=html

Comment 83

[Håkan Waara]

This is a long bug, so I'm not sure if this is covered here; but remember that (if we're aiming for full platform support) the "full keyboard access" option on OS X should toggle whether some elements (like trees) can be focusable at all.

Comment 84

[[e-mail]]

(In reply to comment #57)
> When clicking on HTML inputs, on all platforms: A
What about other HTML form controls?

Comment 85

[Alex Faaborg [:faaborg] (Firefox UX)]

>What about other HTML form controls?

This is a little complicated in that it merges chrome and content controls into a single column, but I think this is the correct approach for each control on each platform.  (would love to get feedback if anyone disagrees with anything):

http://spreadsheets.google.com/pub?key=t8sBdbRxc_OGAZavmb-5F9g&output=html

Comment 86

[Neil Deakin]

Attachment: add some tests

Test changes:
  - window_focus.xul changes because elements are no longer focusable with the mouse on Mac
  - test_focusrings.xul tests whether focus rings appear in various cases
  - /toolkit/content/tests/widgets/test_focus.xul is redundant so I removed it

From the previous patch, this patch changes the windows handling of the WM_CHANGEUISTATE and WM_UPDATEUISTATE messages to be more correct.

Comment 87

[Neil Deakin]

Comment on attachment 405117 [details] [diff] [review]
add some tests

Theme changes

Comment 88

[Neil Deakin]

Comment on attachment 405117 [details] [diff] [review]
add some tests

Windows changes

Comment 89

[Dão Gottwald [:dao]]

Comment on attachment 405117 [details] [diff] [review]
add some tests

I find it slightly irritating that clicking a link doesn't give me immediate feedback anymore. Bug 482985 makes this worse.

>--- a/toolkit/themes/winstripe/global/menulist-aero.css
>+++ b/toolkit/themes/winstripe/global/menulist-aero.css

> menulist:focus > .menulist-label-box {
>-  border: 1px dotted ThreeDDarkShadow;
>   background-color: transparent;
>   color: inherit;
> }

This rule can be removed.

>+menulist:-moz-focusring > .menulist-label-box {
>+  border: 1px dotted ThreeDDarkShadow;
>+}

This should use menulist:-moz-focusring:not([open="true"]):not(.menulist-compact) > .menulist-label-box

>--- a/toolkit/themes/winstripe/global/menulist.css
>+++ b/toolkit/themes/winstripe/global/menulist.css

> .menulist-label-box,
>-menulist[open="true"]:focus > .menulist-label-box {
>+menulist[open="true"]:-moz-focusring > .menulist-label-box {
>   border: 1px solid transparent;
>   background-color: transparent;
>   color: inherit;
>   -moz-box-align: center;
> }

The menulist[open="true"]:-moz-focusring > .menulist-label-box selector can be dropped.

> menulist:focus > .menulist-label-box {
>+  background-color: Highlight;
>+  color: HighlightText;
>+}

This should use menulist:focus:not([open="true"]):not(.menulist-compact) > .menulist-label-box

>+menulist:-moz-focusring > .menulist-label-box {
>   border: 1px dotted #F5DB95;
>-  background-color: Highlight;
>-  color: HighlightText;  
> }

This should use :not([open="true"]) too.

> .menulist-compact:focus > .menulist-label-box {
>-  border: 1px dotted;
>   background-color: transparent;
>   color: inherit;
> }

This rule can be dropped.

>+.menulist-compact:-moz-focusring > .menulist-label-box {
>+  border: 1px dotted;
>+}

:not([open="true"]) as well

You also need to adjust menulist > menupopup > menuitem[_moz-menuactive="true"] in menu.css.

Comment 90

[David Baron :dbaron:]

Was removing test_focus.xul from the makefile intentional?

Comment 91

[Neil Deakin]

Yes, as stated in comment 89, that test is redundant. Not sure why the patch diff didn't pick up the deleted file but it is removed locally.

Comment 92

[Jim Mathies [:jimm]]

+::SendMessageW(mWnd, WM_CHANGEUISTATE, UIS_INITIALIZE, 0);

+  case WM_CHANGEUISTATE:
+  case WM_UPDATEUISTATE:
+  {
+    PRInt32 action = LOWORD(wParam);
+    PRInt32 flags = HIWORD(wParam);
+    // If the UI state has changed or the state is being initialized, update
+    // the UI state. For initialization, this will set the keyboard cues based
+    // on how the window was opened. For example, a dialog opened via a
+    // keyboard press on a button should enable cues, whereas the same dialog
+    // opened via a mouse click of the button should not.
+    if (msg == WM_UPDATEUISTATE || action == UIS_INITIALIZE) {
+      nsUIStateChangeEvent event(PR_TRUE, NS_UISTATECHANGED, this);
+      if (action == UIS_CLEAR) {
+        if (flags & UISF_HIDEACCEL)
+          event.showAccelerators = UIStateChangeType_Set;
+        if (flags & UISF_HIDEFOCUS)
+          event.showFocusRings = UIStateChangeType_Set;
+      }
+      else if (action == UIS_SET) {
+        if (flags & UISF_HIDEACCEL)
+          event.showAccelerators = UIStateChangeType_Clear;
+        if (flags & UISF_HIDEFOCUS)
+          event.showFocusRings = UIStateChangeType_Clear;
+      }
+      DispatchWindowEvent(&event);
+    }
+    else if (action == UIS_SET || action == UIS_CLEAR) {
+      ::SendMessageW(mWnd, WM_UPDATEUISTATE, wParam, lParam);
+    }
+
+    break;
+  }

Aren't we going to generate an bad event here when we send WM_CHANGEUISTATE w/UIS_INITIALIZE to mWnd? If I'm reading this right, when we receive our initial WM_CHANGEUISTATE, we create a nsUIStateChangeEvent, but none of the action flags are set, so we send off an event that isn't initialized properly.

Shouldn't we send WM_CHANGEUISTATE w/UIS_INITIALIZE, let that fall through, and rely on our response to the WM_UPDATEUISTATE broadcast by the system to update?

I'm pulling this from what I read on Chen's blog:

http://blogs.msdn.com/oldnewthing/archive/2005/05/03/414317.aspx

Comment 93

[Neil Deakin]

> Shouldn't we send WM_CHANGEUISTATE w/UIS_INITIALIZE, let that fall through, and
> rely on our response to the WM_UPDATEUISTATE broadcast by the system to update?

The issue here is that when receieve WM_CHANGEUISTATE we send WM_UPDATEUISTATE to all children, but the toplevel window itself never receives a WM_UPDATEUISTATE event.

I'll continue to investigate as I've found a minor issue with initializing.

Comment 94

[Neil Deakin]

Attachment: fix up menulist styles, and simplify windows state message handling

This patch simplifies the handling of the WM_CHANGEUISTATE and WM_UPDATEUISTATE messages, as we can just rely on the system to do most of the work for us.

Comment 95

[Jim Mathies [:jimm]]

Comment on attachment 409969 [details] [diff] [review]
fix up menulist styles, and simplify windows state message handling

r+ on the windows bits.

Comment 96

[[e-mail]]

Comment on attachment 409969 [details] [diff] [review]
fix up menulist styles, and simplify windows state message handling

>+                     newFocusedElement.getAttributeNS("http://www.w3.org/1999/xlink", "type") == "simple"))
Didn't we drop xlink support, or was I imagining it??

> 
>+
I would have thought one blank line was enough ;-)

Comment 97

[Boris Zbarsky [:bzbarsky]]

> Didn't we drop xlink support, or was I imagining it??

We dropped generic xlink.

Of course that still leaves <svg:a> (which uses xlink) and some MathML stuff (which uses XLink but doesn't work right now).  You could use an instanceof test to detect <svg:a>.  This code also doesn't detect <area> or <link> elements; it probably should, no?

We should really have a single scriptable interface that all links implement...  Even if it's completely empty and just used for instanceof.

Comment 98

[Alex Faaborg [:faaborg] (Firefox UX)]

While this change was intended primarily for desktop Firefox, Limi correctly points out that for mobile devices, having visual feedback directly on a link press (for instance the iPhone overlays a grey box on the target you touch) is pretty important feedback.  So Fennec will likely want to make some modifications to the work being done here for touch feedback in the content area.

Comment 99

[David Baron :dbaron:]

(In reply to comment #91)
> Yes, as stated in comment 89, that test is redundant. Not sure why the patch
> diff didn't pick up the deleted file but it is removed locally.

If it doesn't show up in the patch, then I doubt it will get removed when you push, so you should figure out why it's not.


I'm a little concerned about the handling of dynamic changes with this patch, and also its interaction with the new approach to pseudoclass implementation in bug 525952.

Would it make more sense to have a second event state (NS_EVENT_STATE_FOCUS_RING?) that's an element has whenever both of the following are true:
 * the element has NS_EVENT_STATE_FOCUS
 * window->ShouldShowFocusRing() returns true
and then send appropriate ContentStatesChanged notifications for that state?

I'm curious what bzbarsky thinks about that approach, or about the current one.


Sorry for the delay in getting to this.

Comment 100

[Neil Deakin]

Attachment: use separate flag

This version is similar but includes the removed test changes, switches to using a different state flag and adds some extra calls to ensure that the keyboard indicator state is propagated to child windows.

Note that Windows only seems to fire ui update notifications when a window is opened. Changing the preference in the control panel only affects new windows.

Comment 101

[David Baron :dbaron:]

Comment on attachment 428275 [details] [diff] [review]
use separate flag

In the future, when you're requesting review on part of a patch, could
you split the patch into appropriate pieces so that you're requesting review
on a complete diff file and not part of it?

nsEventStateManager.cpp:

>+  nsIContent* focusedContent = fm ? fm->GetFocusedContent() : nsnull;
>+  if (aContent == focusedContent) {
>     aState |= NS_EVENT_STATE_FOCUS;
>+
>+    if (focusedContent) {

Since aContent is known to be non-null, and focusedContent is known to
be equal to aContent, you don't need to null-check focusedContent here.

>+      nsIDocument* doc = focusedContent->GetDocument();

GetDocument() is deprecated; please use GetCurrentDoc() or GetOwnerDoc().
(GetCurrentDoc() is the same as GetDocument(), GetOwnerDoc() is
a hair faster, and in this case I think the difference doesn't matter.)

(Same in nsNativeTheme.cpp.)


>+  if ((aState & NS_EVENT_STATE_FOCUS) || (aState & NS_EVENT_STATE_FOCUSRING)) {

if (aState & (NS_EVENT_STATE_FOCUS | NS_EVENT_STATE_FOCUSRING)) {

That said, what does it mean to call SetContentState(foo,
NS_EVENT_STATE_FOCUSRING).  Might it make more sense to assert that
NS_EVENT_STATE_FOCUSRING is not allowed in the bits passed to
SetContentState, but instead always add it when NS_EVENT_STATE_FOCUS is
passed in?  (And undo the corresponding changes to nsFocusManager?) But
I guess that wouldn't work with the nsGlobalWindow changes, so probably
ignore this comment.

nsGlobalWindow.cpp:

+    }
+    else if (aFocusMethod & nsIFocusManager::FLAG_SHOWRING

local style is "} else" on same line

>+      if (childWindow) {
>+               childWindow->SetKeyboardIndicators(aShowAccelerators, aShowFocusRings);
>+      }

Weird indentation here (tabs)?

In nsGlobalWindow::SetKeyboardIndicators, is the change intended to
propagate across chrome/content boundaries?

>+  nsPIDOMWindow* root = GetPrivateRoot();
>+  if (root) {
>+       PRBool showAccelerators, showFocusRings;
>+       root->GetKeyboardIndicators(&showAccelerators, &showFocusRings);
>+       mShowFocusRings = showFocusRings;
>+  }

2 space indent, not 5




I wonder whether we should really be introducing :-moz-focusring, or
whether it would be better to apply the difference to :focus.  I think
I'd actually slightly prefer applying the changes to :focus instead.  It
might be worth seeing what others think (maybe bzbarsky, or perhaps even
the www-style list.)

If we apply the differences to :focus, then Web pages writing :focus
styles will do what matches the UI.  That said, there could be some edge
cases where they really want the distinction.

r=dbaron either way, though, though please do bring up the issue of
:focus vs. :moz-focusring with others and get some additional opinions.
Sorry for the delay in getting to this.

Comment 102

[Neil Deakin]

bz preferred the current approach, so I checked in the patch as is and we can evaluate feedback. I made one minor fix so that default value of showing indicators on mac is false.

http://hg.mozilla.org/mozilla-central/rev/f3abfe490d7f

Comment 103

[David Baron :dbaron:]

Looks like mochitest-plain 3/5 has been orange on Mac since this landed.  I presume somebody will back it out soon unless that's fixed.

Comment 104

[Boris Zbarsky [:bzbarsky]]

> bz preferred the current approach

Specifically, because the common use of :focus I've noticed on the web is for highlighting input fields, and the sites definitely want those highlighted when you click on them.

Comment 105

[Alfred Kayser]

Can someone explain how to make a theme compatible with both :focus and :-moz-focusring? Just doing both :focus and :-moz-focusring won't work. FF older than 3.7 should use :focus, but >=3.7 should use -moz-focusring if I understand correctly. How to do this in a theme that supports both <FF3.7 and >=FF3.7?

Comment 106

[Danny]

If you click on link, wait until the other page is loaded, then click "Back" browser button and go back to the original page, the focus ring will appear around link. Is it bug or intended behavior?

Comment 107

[Neil Deakin]

That's intended behaviour, so that you can see which link you last clicked. For example, see comment 8.

Comment 108

[Jim Mathies [:jimm]]

(In reply to comment #107)
> That's intended behaviour, so that you can see which link you last clicked. For
> example, see comment 8.

intended and very useful!

Comment 109

[Daniel Luz]

A few problems I've noticed on OS X:

- Issues reported on comment #75 remain.
- Open the proxy settings, click "Manual proxy configuration", then click any of its text boxes. Now, click "Automatic proxy configuration"; the text box will remain focused even though it is no longer enabled. Click the automatic proxy configuration URL text box, the previous text box will keep a focus ring even though it is no longer focused.
- When full keyboard access (System Preferences) is enabled, the focus ring should appear around chrome buttons. They still shouldn't get focus from clicks, but this is important on confirmation sheets. For example, open TextEdit, type something on a blank document, then try to close it. The "Don't Save" button has a focus ring, and this is extremely important so that you know what happens if you press the space bar. The analogous situation in Firefox is when you try to close a window or quit the application, and a confirmation appears. It's important to know that "Cancel" (or "Quit") is the action taken when you press space bar.

Comment 110

[Neil Deakin]

Don't report additional problems in this bug, otherwise they will get lost. Regressions or followup bugs should be new bugs and be marked as blocking this bug.

Comment 111

[Danny]

Focus rings are suddenly back with the latest build.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100425 Minefield/3.7a5pre

Comment 112

[Eric Shepherd [:sheppy]]

Now documented here:

https://developer.mozilla.org/en/CSS/:-moz-focusring

Let me know if there are any issues with that doc.

Comment 114

[709922234]

spec name: https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo

Comment 115

[Boris Zbarsky [:bzbarsky]]

Bug 1437901 is tracking that.

Comment 116

[Boris Zbarsky [:bzbarsky]]

Er, sorry, bug 1445482.