Bug 61421

Summary: Handle focus event in shadow content.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: DOMAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, hayato, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 59805, 61409    
Attachments:
Description Flags
WIP: Needs tests and loving.
none
patch none

Description Hayato Ito 2011-05-24 22:59:19 PDT
We should make sure that focus event happend in elements of shadow content is properly retargeted to a right element, which should be a shadow host element, and call right eventListners.
Comment 1 Dimitri Glazkov (Google) 2011-06-08 16:38:31 PDT
Created attachment 96506 [details]
WIP: Needs tests and loving.
Comment 2 Hayato Ito 2011-08-17 05:10:31 PDT
Created attachment 104166 [details]
patch
Comment 3 Hayato Ito 2011-08-17 05:12:18 PDT
Hi Dimitri,
I've updated a patch, which is based on your patch, fixing some minor issues and adding a LayoutTest.

Could you review that?
Comment 4 Dimitri Glazkov (Google) 2011-08-17 08:31:42 PDT
Comment on attachment 104166 [details]
patch

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

> Source/WebCore/dom/Event.cpp:343
> +PassRefPtr<FocusEventDispatchMediator> FocusEventDispatchMediator::create(PassRefPtr<Node> oldFocusedNode)
> +{
> +    return adoptRef(new FocusEventDispatchMediator(oldFocusedNode));
> +}
> +
> +FocusEventDispatchMediator::FocusEventDispatchMediator(PassRefPtr<Node> oldFocusedNode)
> +    : EventDispatchMediator(Event::create(eventNames().focusEvent, false, false))
> +    , m_oldFocusedNode(oldFocusedNode)
> +{
> +}
> +
> +bool FocusEventDispatchMediator::dispatchEvent(EventDispatcher* dispatcher) const
> +{
> +    dispatcher->adjustRelatedTarget(event(), m_oldFocusedNode);
> +    return EventDispatchMediator::dispatchEvent(dispatcher);
> +}
> +
> +PassRefPtr<BlurEventDispatchMediator> BlurEventDispatchMediator::create(PassRefPtr<Node> newFocusedNode)
> +{
> +    return adoptRef(new BlurEventDispatchMediator(newFocusedNode));
> +}
> +
> +BlurEventDispatchMediator::BlurEventDispatchMediator(PassRefPtr<Node> newFocusedNode)
> +    : EventDispatchMediator(Event::create(eventNames().blurEvent, false, false))
> +    , m_newFocusedNode(newFocusedNode)
> +{
> +}
> +
> +bool BlurEventDispatchMediator::dispatchEvent(EventDispatcher* dispatcher) const
> +{
> +    dispatcher->adjustRelatedTarget(event(), m_newFocusedNode);
> +    return EventDispatchMediator::dispatchEvent(dispatcher);
> +}

These should be in UIEvent, right?

> Source/WebCore/page/FocusController.cpp:80
> +        document->focusedNode()->dispatchBlurEvent(0);

Ah. This is a nice improvement over my old patch! :)
Comment 5 Hayato Ito 2011-08-17 17:29:58 PDT
Comment on attachment 104166 [details]
patch

Thank you for the review.

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

>> Source/WebCore/dom/Event.cpp:343
>> +}
> 
> These should be in UIEvent, right?

Focus/Blur events are not UIEvent in definition according to http://www.w3.org/TR/DOM-Level-2-Events/events.html.
Hence, I'd like to avoid putting these in UIEvent.{h,cpp}, even though they look very 'similar' to FocusIn/FocusOut events.

I am landing this patch as is. But it's uncomfortable for me to leave these Focus/BlurEventDispatchMediator in Event.h.  So I'll extract EventDispatchMediator and Focus/BlueEventDispatchMediator to another files, named EventDispatchMediator.{h, cpp} in another patch.
Comment 6 WebKit Review Bot 2011-08-17 19:41:25 PDT
Comment on attachment 104166 [details]
patch

Clearing flags on attachment: 104166

Committed r93276: <http://trac.webkit.org/changeset/93276>
Comment 7 WebKit Review Bot 2011-08-17 19:41:29 PDT
All reviewed patches have been landed.  Closing bug.