Bug 95169 – [ShadowDOM] Shadow elements in the input element should be focusable.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

The implementation of multiple fields time input UI wants to have focusable shadow elements for focus navigation.
For this purpose, FocusController should walk through shadow elements for "time" input type. 

Although, current implementation of focus navigation in FocusController class introduced by r112511 treats all
input types in same way.

Created attachment 160898 [details]
Patch 1

Comment on attachment 160898 [details]
Patch 1

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 160898 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=160898&action=review

> Source/WebCore/html/HTMLAudioElement.h:51
> +    virtual bool hasCustomFocusLogic() const OVERRIDE;

This should be defined at HTMLMediaElement instead of HTMLAudioElement and HTMLVideoElement because HTMLMediaElement is responsible to build shadow sub tree for media control UI.

Created attachment 160901 [details]
Patch 2

Comment on attachment 160901 [details]
Patch 2

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Add hasCustomFocusLogic() to HTMLMediaElement instead of HTMLAudioElement and HTMLVideoElement

[Kent Tamura]

Comment on attachment 160901 [details]
Patch 2

ok

Comment on attachment 160901 [details]
Patch 2

Clearing flags on attachment: 160901

Committed r126842: <http://trac.webkit.org/changeset/126842>

All reviewed patches have been landed.  Closing bug.

[Eric Carlson]

(In reply to comment #5)
> (From update of attachment 160901 [details])
> * Add hasCustomFocusLogic() to HTMLMediaElement instead of HTMLAudioElement and HTMLVideoElement

What is "multiple fields time input UI" and what does it have to do with HTMLMediaElement?

(In reply to comment #9)
> (In reply to comment #5)
> > (From update of attachment 160901 [details] [details])
> > * Add hasCustomFocusLogic() to HTMLMediaElement instead of HTMLAudioElement and HTMLVideoElement
> 
> What is "multiple fields time input UI"?
Multiple fields time input UI is introduced by bug 88970.
It is text field like UI but it has fields for hour, minute and second. It is similar to platform provided DateTime control.

>what does it have to do with HTMLMediaElement?
"audio" and "video" elements uses shadow DOM for implementing their UI, focus controller hasn't supported their detail of focus navigation yet. "input" element is same situation. But, I would like to override this focus navigation behavior based on input type.

[Eric Carlson]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #5)
> > 
> > What is "multiple fields time input UI"?
> Multiple fields time input UI is introduced by bug 88970.
> It is text field like UI but it has fields for hour, minute and second. It is similar to platform provided DateTime control.
> 
> >what does it have to do with HTMLMediaElement?
> "audio" and "video" elements uses shadow DOM for implementing their UI, focus controller hasn't supported their detail of focus navigation yet. "input" element is same situation. But, I would like to override this focus navigation behavior based on input type.

I am sorry, but I still don't understand - maybe because I don't know what the focus controller or focus navigation are. Can you please explain in more detail or point me to something I can read to educate myself?

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #5)
> > > 
> > > What is "multiple fields time input UI"?
> > Multiple fields time input UI is introduced by bug 88970.
> > It is text field like UI but it has fields for hour, minute and second. It is similar to platform provided DateTime control.
> > 
> > >what does it have to do with HTMLMediaElement?
> > "audio" and "video" elements uses shadow DOM for implementing their UI, focus controller hasn't supported their detail of focus navigation yet. "input" element is same situation. But, I would like to override this focus navigation behavior based on input type.
> 
> I am sorry, but I still don't understand - maybe because I don't know what the focus controller or focus navigation are. Can you please explain in more detail or point me to something I can read to educate myself?

Actually, I'm not familiar with FocusController. When I study how Shift+Tab handled, I saw page/EventHandler.cpp, then I reached page/FocusController.cpp

My trouble was FocusController didn't walk into shadow DOM tree hosted by "input" element. My experiment proofs FocusController walks into the "span" element. Simple search "input" gave me what I need to change.

[Hayato Ito]

Let me comment briefly. I'll take a look at the patch tomorrow.

When I supported Shadow DOM in regard to focus navigations, I found that input element has a special focus logic. That caused me to exclude input elements from general focus navigation. So we cannot enter a shadow DOM of input elements in focus navigations.

I think we need to update input elements somehow before we get rid of such special treatment. Let me take a look at the patch tomorrow.

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > (In reply to comment #5)
> > > > 
> > > > What is "multiple fields time input UI"?
> > > Multiple fields time input UI is introduced by bug 88970.
> > > It is text field like UI but it has fields for hour, minute and second. It is similar to platform provided DateTime control.
> > > 
> > > >what does it have to do with HTMLMediaElement?
> > > "audio" and "video" elements uses shadow DOM for implementing their UI, focus controller hasn't supported their detail of focus navigation yet. "input" element is same situation. But, I would like to override this focus navigation behavior based on input type.
> > 
> > I am sorry, but I still don't understand - maybe because I don't know what the focus controller or focus navigation are. Can you please explain in more detail or point me to something I can read to educate myself?
> 
> Actually, I'm not familiar with FocusController. When I study how Shift+Tab handled, I saw page/EventHandler.cpp, then I reached page/FocusController.cpp
> 
> My trouble was FocusController didn't walk into shadow DOM tree hosted by "input" element. My experiment proofs FocusController walks into the "span" element. Simple search "input" gave me what I need to change.