Rudimentary keyboard navigation support for <video>/<audio>

Rudimentary keyboard navigation support for <video>/<audio>

Remove the override of hasCustomFocusLogic() for HTMLMediaElement. This
allows focus navigation (<tab>ing) to reach the controls.

BUG=135661
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174795

[fs, 2014-05-20 17:04:38]

Starting change to get some form of keyboard navigation going. The "auto-hide"
functionality is still trouble-some, but I'll post a CL with a heuristic to
attempt to avoid that after this - which might warrant more discussion - so
wanted this part out of the way first if possible. PTAL.

[philipj_slow, 2014-05-21 08:48:30]

Have you tracked down the history of why hasCustomFocusLogic() was there in the
first place?

[fs, 2014-05-21 12:04:57]

On 2014/05/21 08:48:30, philipj wrote:
> Have you tracked down the history of why hasCustomFocusLogic() was there in
the
> first place?

Yes, it was added in r126842 / e315fe5fdb19315d6b8536675a51740c29579ce9, where
it was essentially "moved" from a static method in FocusController (checking on
tagname) to different subclasses of HTMLElement. That static method came to be
in r90004 / 6ff7d3021245765344d39a1cbd9ead86057ef621 which I believe concerned
focus-navigation in shadow trees:

"This patch doesn't take an effect on the following elements to keep a
compatibility.
 - <input>, <textarea>, <video> and <audio> elements
We'll address these elements separately after re-targeting focus events
(bug 61421) is implemented."

(The referenced WK bug #61421 was fixed in r93276 /
3c959975feb261832eecee34916fa439461ead23)

[philipj_slow, 2014-05-21 13:50:01]

OK, another todo that was never done :) My only concern here is that without
preventing the controls from fading out when they have keyboard focus, the
result is something that looks buggier than no support at all. LGTM if you
intent to land the next patch before long.

[fs, 2014-05-21 13:58:10]

On 2014/05/21 13:50:01, philipj wrote:
> OK, another todo that was never done :) My only concern here is that without
> preventing the controls from fading out when they have keyboard focus, the
> result is something that looks buggier than no support at all. LGTM if you
> intent to land the next patch before long.

Yes, that was the intention... I'm not sure about "buggier" though since with
this patch alone you can actually reach the controls at all.
This and the follow-up is independent of each other though, so it should be
possible to hold on to this one until they're both reviewed and ready to commit
(and then land them close together). Or I could just merge them if you feel that
having one without the other will not be an incremental improvement.

[philipj_slow, 2014-05-21 14:28:02]

I haven't compiled and tested this patch, I'm just guessing that the controls
will fade away "accidentally" on mouseout or similar.

Since the next patch is forthcoming it hardly matters how this is split, so
LGTM.

Leaving the final LGTM to Aaron.

[fs, 2014-05-21 15:07:53]

On 2014/05/21 14:28:02, philipj wrote:
> I haven't compiled and tested this patch, I'm just guessing that the controls
> will fade away "accidentally" on mouseout or similar.

Yes, that's pretty much what will happen (on playback).
 
> Since the next patch is forthcoming it hardly matters how this is split, so
> LGTM.

I uploaded the tentative heuristic to https://codereview.chromium.org/297783004
in case you want to have a look at it.

[philipj_slow, 2014-05-21 22:11:25]

On 2014/05/21 15:07:53, fs wrote:

> I uploaded the tentative heuristic to
https://codereview.chromium.org/297783004
> in case you want to have a look at it.

That looks sensible at first glance, I'll review it more carefully once
submitted.

[acolwell GONE FROM CHROMIUM, 2014-05-22 00:21:36]

lgtm

[fs, 2014-05-26 07:36:30]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-05-26 07:36:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fs@opera.com/292173006/1

[commit-bot: I haz the power, 2014-05-26 09:04:16]

Change committed as 174795