[ContentChangeObserver] Dispatch synthetic mouse event asynchronously in completePendingSyntheticClickForContentChangeObserver

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[zalan]

<rdar://problem/51787961>

[zalan]

Created attachment 372901 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 372901 [details]
Patch

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

> Source/WebKit/ChangeLog:10
> +        WebPage::completePendingSyntheticClickForContentChangeObserver should not dispatch mouse events synchronously as through style updates
> +        they could destroy the element that initiated this change. WebPage::handleSyntheticClick() already implements this pattern. 

"as through style updates" is hard to parse. Break into two sentences?

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:720
>  

Should you check if the targetNode is still connected?

[zalan]

Created attachment 372903 [details]
Patch

[zalan]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 372901 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372901&action=review
> 
> > Source/WebKit/ChangeLog:10
> > +        WebPage::completePendingSyntheticClickForContentChangeObserver should not dispatch mouse events synchronously as through style updates
> > +        they could destroy the element that initiated this change. WebPage::handleSyntheticClick() already implements this pattern. 
> 
> "as through style updates" is hard to parse. Break into two sentences?
> 
> > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:720
> >  
> 
> Should you check if the targetNode is still connected?
Not sure. Will check with rniwa tomorrow.

[Ryosuke Niwa]

Comment on attachment 372903 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:724
> +            protectedThis->completeSyntheticClick(targetNode, location, modifiers, WebCore::OneFingerTap, pointerId);

Yes, we should check that targetNode is still in the same document.
Dispatching mouse events on a detached node would be weird.

[zalan]

Created attachment 372921 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 372921 [details]
Patch

Clearing flags on attachment: 372921

Committed r246835: <https://trac.webkit.org/changeset/246835>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.