Bug 108447

Summary: INPUT_MULTIPLE_FIELDS_UI: Focus order is not controllable by tabIndex attribute on <input>
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, dglazkov, hayato, mifenton, morrita, ojan.autocc, webkit.review.bot, yosin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 108775    
Bug Blocks: 108795    
Attachments:
Description Flags
WIP
none
Patch none

Description Kent Tamura 2013-01-31 02:44:40 PST
<body>
<input tabindex=30 value=30>
<input tabindex=20 type=date>
<input tabIndex=10 value=10 autofocus>
</body>

Steps to reproduce:
 1. Open the document above on Google Chrome 24+.
 2. Press TAB 3 times
   ==> The last sub-field in the date input is focused.
 3. Press TAB

Expected:
  The input with tabindex=30 is focused.

Actual:
  The 'Back' navigation button is focused.


The shadow host of the date input is not focusable, and it's shadow sub-fields are focusable. We have no ways to control focus sequence in this case though the shadow host has tabindex.

Possible solutions:
  A) Introduce special handling to respect tabindex on non-focusable shadow host
  B) Make the shadow host <input> focusable
     In this case, we'd like to focus on the first sub-field when the shadow host is focused, and Shift+TAB on the first sub-field should skip focusing on the shadow host. We need another hack to detect focus direction.
  C) Stop using shadow DOM focus navigation.
    Make the shadow host <input> focusable, and make sub-fields non-focusable.
    We can emulate the current behavior by detecting focus direction.
Comment 1 Dimitri Glazkov (Google) 2013-01-31 09:06:36 PST
Just to make sure I understand, let me try to say it back to you in a slightly different way:

We need a way to indicate that some shadow hosts must never participate in sequential focus navigation, delegating their focusing completely to the tree they host.

Right?
Comment 2 Kent Tamura 2013-01-31 16:04:34 PST
(In reply to comment #1)
> We need a way to indicate that some shadow hosts must never participate in sequential focus navigation, delegating their focusing completely to the tree they host.
> 
> Right?

I think so.
I feel the current rule for focusable-shadow-nodes-in-non-focusable-shadow-host has a defect because component users have no ways to specify tabindex to a component.  We might want to introduce a new attribute for shadow hosts like shadow-tree-tab-index=20.

Anyway, we need a hack for input[type=date] because it should work with normal tabindex attribute.
Comment 3 Hayato Ito 2013-01-31 18:19:35 PST
(In reply to comment #0)
> <body>
> <input tabindex=30 value=30>
> <input tabindex=20 type=date>
> <input tabIndex=10 value=10 autofocus>
> </body>
> 
> Steps to reproduce:
>  1. Open the document above on Google Chrome 24+.
>  2. Press TAB 3 times
>    ==> The last sub-field in the date input is focused.
>  3. Press TAB
> 
> Expected:
>   The input with tabindex=30 is focused.
> 
> Actual:
>   The 'Back' navigation button is focused.
> 
> 
> The shadow host of the date input is not focusable, and it's shadow sub-fields are focusable. We have no ways to control focus sequence in this case though the shadow host has tabindex.
> 
> Possible solutions:
>   A) Introduce special handling to respect tabindex on non-focusable shadow host
>   B) Make the shadow host <input> focusable
>      In this case, we'd like to focus on the first sub-field when the shadow host is focused, and Shift+TAB on the first sub-field should skip focusing on the shadow host. We need another hack to detect focus direction.

B) was what I had implemented before. I called it 'focus delegation'.
But I've removed that to prefer a general rule defined in the spec.

I think we need a hack to keep a compatibility for built-in elements, such as input elements, which has a shadow tree unless we can have a good spec to cover this case.


>   C) Stop using shadow DOM focus navigation.
>     Make the shadow host <input> focusable, and make sub-fields non-focusable.
>     We can emulate the current behavior by detecting focus direction.
Comment 4 Kent Tamura 2013-01-31 20:12:42 PST
I discussed this with some people offline, and concluded we should take B.

A is not good because we need to add a hack to FocusController only for INPUT_MULTIPLE_FIELDS_UI.
C is not good because we'd like to expose aria-* attributes on sub-fields. If sub-fields were not focusable, WebCore a11y would not expose them.

To implement B in HTMLInputElement, I'd like to add FocusDirection argument to Node::setFocus.
Comment 5 Kent Tamura 2013-02-01 01:47:41 PST
Created attachment 185981 [details]
WIP
Comment 6 Build Bot 2013-02-01 02:39:35 PST
Comment on attachment 185981 [details]
WIP

Attachment 185981 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16309541
Comment 7 Build Bot 2013-02-01 03:05:08 PST
Comment on attachment 185981 [details]
WIP

Attachment 185981 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16298545
Comment 8 Kent Tamura 2013-02-03 23:55:10 PST
Created attachment 186315 [details]
Patch
Comment 9 Build Bot 2013-02-04 01:10:34 PST
Comment on attachment 186315 [details]
Patch

Attachment 186315 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16377009
Comment 10 Kent Tamura 2013-02-04 01:12:23 PST
Comment on attachment 186315 [details]
Patch

(In reply to comment #9)
> (From update of attachment 186315 [details])
> Attachment 186315 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/16377009

false alarm.
Comment 11 Hajime Morrita 2013-02-04 15:37:58 PST
Comment on attachment 186315 [details]
Patch

r=me
Comment 12 WebKit Review Bot 2013-02-04 17:18:47 PST
Comment on attachment 186315 [details]
Patch

Clearing flags on attachment: 186315

Committed r141835: <http://trac.webkit.org/changeset/141835>
Comment 13 WebKit Review Bot 2013-02-04 17:18:51 PST
All reviewed patches have been landed.  Closing bug.