Mouse press should focus on any types of form controls.

Mouse press should focus on any types of form controls.

In WebKit, form controls except text fields and <select> are not
mouse-focusable. IE, Firefox, and Opera(Presto) don't have such behavior
and they set focus by mouse press. We follow non-WebKit browsers.

The main part of this CL is to remove
HTMLFormControlElement::isMouseFocusable. It means we use
Node::isMouseFocusable, which is just isFocusable. We can remove many
isMouseFocusable implementations of HTMLFormControlElement 
subclasses.
    
Test changes:
- Remove fast/events/click-focus-control.html because now it is covered
   by existing and new tests.
- Need rebaseline for some tests. They contain focused form controls.
    
BUG=89708

[keishi, 2013-04-15 11:59:04]

On 2013/04/15 09:00:29, tkent wrote:

I thought we would be keeping the existing behavior for Mac. I tried this patch
and I think the Mac users will notice.

[tkent, 2013-04-15 22:21:34]

> I thought we would be keeping the existing behavior for Mac. I tried this
patch
> and I think the Mac users will notice.

I re-checked the other browser behavior, and found Firefox/Mac behaved same as
Safari and the current Chromium and Opera/Mac behaved same as non-WebKit WIndows
browsers.

IMO, we should apply this change on Mac too for web-compatibility. We might want
to hide focus rings in a case of focus-by-click.

[ojan, 2013-04-15 22:36:16]

lgtm

I think the benefits to web developers of getting a consistent platform outweigh
the cost of not matching the platform here.

[eseidel, 2013-04-15 23:11:28]

Could you explain what behavioral different mac users will experiance in Chrome?

[keishi, 2013-04-16 01:58:24]

On 2013/04/15 23:11:28, Eric Seidel (Google) wrote:
> Could you explain what behavioral different mac users will experiance in
Chrome?

What stood out most for me is the fact that buttons get focus when clicked.
Another thing I'd like to point out is that we can use onclick="this.focus()" to
implement this behavior cross platform but it would be very hard to get the
current behavior using JS.

[tkent, 2013-04-16 02:07:08]

> What stood out most for me is the fact that buttons get focus when clicked.

Do you mean just focus ring appearance, or keyboard input acceptance?

> Another thing I'd like to point out is that we can use onclick="this.focus()"
to
> implement this behavior cross platform but it would be very hard to get the
> current behavior using JS.

We can prevent focus by event.preventDefault() in mousedown event handler.

[keishi, 2013-04-16 03:21:09]

On 2013/04/16 02:07:08, tkent wrote:
> > What stood out most for me is the fact that buttons get focus when clicked.
> 
> Do you mean just focus ring appearance, or keyboard input acceptance?
I wouldn't expect the focus ring to appear.
> > Another thing I'd like to point out is that we can use
onclick="this.focus()"
> to
> > implement this behavior cross platform but it would be very hard to get the
> > current behavior using JS.
> 
> We can prevent focus by event.preventDefault() in mousedown event handler.

That might also prevent desired behavior like for <input type=range>.

I looked at all my open tabs and I couldn't find a single button that hasn't
been styled (except codereview.chromium.org). And since (I was told) this could
fix a number of bugs I guess the benefits outweigh the cost. lgtm

[Avi (use Gerrit), 2013-04-16 03:55:08]

On 2013/04/15 23:11:28, Eric Seidel (Google) wrote:
> Could you explain what behavioral different mac users will experiance in
Chrome?

Would this change the tab-ordering behavior? Mac users complained loudly when we
got the "full keyboard access" behavior wrong (if it is on, everything could be
tabbed to, and if it is off, you can only tab to text fields and the select
fields).

Why would selection via mouse run by a different mechanism? Also... adding the
ability to focus any element with a mouse click... I don't know if anyone would
notice. Would they get the focus ring? I'm sure Mac users would notice then.

[tkent, 2013-04-16 05:49:02]

On 2013/04/16 03:55:08, Avi wrote:
> Would this change the tab-ordering behavior? Mac users complained loudly when
we
> got the "full keyboard access" behavior wrong (if it is on, everything could
be
> tabbed to, and if it is off, you can only tab to text fields and the select
> fields).

No. This won't change any behavior of TAB focus navigation.

> 
> Why would selection via mouse run by a different mechanism? Also... adding the
> ability to focus any element with a mouse click... I don't know if anyone
would
> notice.

I guess isMouseFocusable was introduced to emulate the Mac-native behavior,
which I'd like to change by this CL.

> Would they get the focus ring? I'm sure Mac users would notice then.

yes.

[Avi (use Gerrit), 2013-04-16 14:16:36]

On 2013/04/16 05:49:02, tkent wrote:
> > Why would selection via mouse run by a different mechanism? Also... adding
the
> > ability to focus any element with a mouse click... I don't know if anyone
> would
> > notice.
> 
> I guess isMouseFocusable was introduced to emulate the Mac-native behavior,
> which I'd like to change by this CL.

a) My question was: Shouldn't you have the same set of items be focusable,
whether by keyboard or mouse? Why did they need a second set of functions?

b) Is this behavior different whether or not "full keyboard access" is set?

> > Would they get the focus ring? I'm sure Mac users would notice then.
> 
> yes.

You're expecting to change a behavior specific to how the Mac works and that's
visible to the user, and not have them notice? :)

[ojan, 2013-04-17 00:33:19]

On 2013/04/16 14:16:36, Avi wrote:
> On 2013/04/16 05:49:02, tkent wrote:
> > > Why would selection via mouse run by a different mechanism? Also... adding
> the
> > > ability to focus any element with a mouse click... I don't know if anyone
> > would
> > > notice.
> > 
> > I guess isMouseFocusable was introduced to emulate the Mac-native behavior,
> > which I'd like to change by this CL.
> 
> a) My question was: Shouldn't you have the same set of items be focusable,
> whether by keyboard or mouse? Why did they need a second set of functions?

If you give an element a negative tabindex it makes it mouse focusable, but not
keyboard focusable. The form controls modified here ignore that and never make
them mouse focusable, even if they have a tabindex. 

> b) Is this behavior different whether or not "full keyboard access" is set?

In Chrome it's not. It is different in Safari.

> > > Would they get the focus ring? I'm sure Mac users would notice then.
> > 
> > yes.
> 
> You're expecting to change a behavior specific to how the Mac works and that's
> visible to the user, and not have them notice? :)

Interestingly, right now on Chrome, if you set a positive tabindex, we make
buttons keyboard focusable regardless of the "full keyboard access" setting.

Ultimately, this is about Chrome providing a consistent experience to web
developers where possible. I'm really skeptical that many users will notice this
change.

[eseidel, 2013-04-17 00:44:06]

To be clear:  My reading above is that with this change, when you click on a
button on Mac, it will draw a focus ring around that button until you click on
something else.  Correct?

Currently we draw no such focus ring on button click.

http://plexode.com/eval3/#s=aelBcA0lVAxsDHUNWVVVQTx9VRlRVHRCVl5kfPU8dVEZNRkRV...

If my understanding is correct, I definitely believe that users would notice
such a change.  Can we obtain the desired platform-stability for web developers
w/o making Mac users feel like their in a non-native environment?

[ojan, 2013-04-18 02:57:29]

Right now, form controls do the default Mac behavior at the expense of
consistency with other platforms, nearly all other browsers and other HTML
elements (e.g. <div tabindex=-1>).

We could change this behavior to be restricted to only be on the Mac and to only
apply if you don't have "full keyboard access" set. I don't think the benefit in
matching the platform is worth fracturing the web platform behavior over though.
Web developers would now have different focus bugs based of whether they're
testing on Chrome Mac or Chrome elsewhere.

[tkent, 2013-04-18 11:47:36]

> > > > Why would selection via mouse run by a different mechanism? Also...
adding
> > the
> > > > ability to focus any element with a mouse click... I don't know if
anyone
> > > would
> > > > notice.
> > > 
> > > I guess isMouseFocusable was introduced to emulate the Mac-native
behavior,
> > > which I'd like to change by this CL.
> > 
> > a) My question was: Shouldn't you have the same set of items be focusable,
> > whether by keyboard or mouse? Why did they need a second set of functions?
> 
> If you give an element a negative tabindex it makes it mouse focusable, but
not
> keyboard focusable. The form controls modified here ignore that and never make
> them mouse focusable, even if they have a tabindex. 
> 
> > b) Is this behavior different whether or not "full keyboard access" is set?
> 
> In Chrome it's not. It is different in Safari.

Wow, I thought Chrome respected the full keyboard access setting, but It
doesn't.
So, we have two sets of form controls for now: keyboard-focusable (everything)
and mouse-focusable (text and <select>).
This patch will merge them into single set.

> > > > Would they get the focus ring? I'm sure Mac users would notice then.
> > > 
> > > yes.
> > 
> > You're expecting to change a behavior specific to how the Mac works and
that's
> > visible to the user, and not have them notice? :)

As Keishi wrote, web authors set their own focus style in major sites.  Users
might see visual difference in very small number of sites.  I don't think many
users complain about this change.

[tkent, 2013-04-18 11:55:52]

On 2013/04/17 00:44:06, Eric Seidel (Google) wrote:
> To be clear:  My reading above is that with this change, when you click on a
> button on Mac, it will draw a focus ring around that button until you click on
> something else.  Correct?

Correct.

> If my understanding is correct, I definitely believe that users would notice
> such a change.  Can we obtain the desired platform-stability for web
developers
> w/o making Mac users feel like their in a non-native environment?

Many sites use styled buttons. Mac users already don't have native feeling in
many sites.

[tkent, 2013-04-25 02:00:15]

If there are no other objections, I'd like to land this patch just after M28
branch.

[ojan, 2013-04-25 02:07:32]

Elliot had some concerns. He was saying that people are annoyed at the Firefox
behavior of showing the dotted border and he pointed out that it would be very
annoying if we did something like this for links. He was saying that we should
instead make buttons, etc the same as divs (i.e. focus them on mouse click if
they have a tabindex).

I still lean towards this patch being the right behavior.

[esprehn, 2013-04-25 03:57:18]

On 2013/04/25 02:07:32, ojan wrote:
> Elliot had some concerns. He was saying that people are annoyed at the Firefox
> behavior of showing the dotted border and he pointed out that it would be very
> annoying if we did something like this for links. He was saying that we should
> instead make buttons, etc the same as divs (i.e. focus them on mouse click if
> they have a tabindex).
> 
> I still lean towards this patch being the right behavior.

I really think we should start with tabindex and then work from there. It seems
wrong for form controls to do this and links not to, Firefox makes everything
focusable and everyone hates their behavior.

[tkent, 2013-04-25 04:52:40]

I don't think the tabindex idea is acceptable for web authors.  It adds the
third behavior which matches to neither WebKit nor non-WebKit.

If the concern is just appearance, how about introducing an internal CSS pseudo
class like
  button:-internal-focus-by-mouse {
      outline: none;
   }
?
(-internal- means it's available only in UA stylesheet.)

[arv (Not doing code reviews), 2013-04-29 23:14:43]

This is a step in the right direction. What we need to do to make sure we do not
show the focus outline when the user clicks on them, we should only show the
outline when the user starts to use the keyboard to tab between elements (or if
tabindex is set). This is what IE does.

[tkent, 2013-05-23 02:28:56]

I tried to implement the followings:

* HTMLFormControlElement has a flag to represent focused-by-mouse.
* Introduce :-internal-focus-by-mouse and :-internal-focus-by-keyboard pseudo
classes
* UA stylesheet has
:-internal-focus-by-keyboard, :-internal-focus-by-mouse {
    outline: auto 5px -webkit-focus-ring-color;
}
button:-internal-focus-by-mouse, ... {
    outline: none;
}

Implementing them is straight-forward. However, if a page author specifies:

:focus {
    outline: solid 2px blue;
}

IMO, this ruleset should override button:-internal-focus-by-mouse ruleset in the
UA stylesheet though button:-internal-focus-by-mouse is more specific than
:focus.
We need ugly hacks in 'specificity' calculation code, and I feel such hacks
would be
harmful in the future.


So my another approach is to avoid to show focus ring only for native appearance
(-webkit-appearance != none).  It would add simple hack to
RenderTheme::isFocused,
and won't update core/css/ files at all.

[tkent, 2013-05-30 06:41:00]

I'm closing this codereview and will make another for new approach because this
has obsolete LGTMs.

[tkent, 2013-05-30 06:53:36]

On 2013/05/30 06:41:00, tkent wrote:
> I'm closing this codereview and will make another for new approach because
this
> has obsolete LGTMs.

New issue is here: https://codereview.chromium.org/16194013/