Bug 113171 – Allow ShadowContents in HitTests by default.

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

[Hayato Ito]

HitTestRequest is widely used in WebCore, but AllowShadowContents flag is not turned on by default.
In most of such places, we can set the flag on.

To track all existing HitTests which does not allow Shadow Contents, and discourage further HitTests which does now allow Shadow Contents in the future, we should turn AllowShadowContents flag on by default.

Instead of AllowShadowContent flag, we should introduce disallowShadowContenet flag so that callers must set this flag on explicitly if they want to disallow Shadow Trees in HitTests,.

This change should be just refactoring and should not include any behavior changes. 
After this change, we'll investigate each place where disallowShadowContents is used and get rid of the flag if it is okay to remove.

[Hayato Ito]

Created attachment 194790 [details]
Refactoring to prevent further HitTests which disallow ShadowContents

[Build Bot]

Comment on attachment 194790 [details]
Refactoring to prevent further HitTests which disallow ShadowContents

Attachment 194790 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17242359

[Hayato Ito]

Let me fix the build of mac-wk2.

(In reply to comment #2)
> (From update of attachment 194790 [details])
> Attachment 194790 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-commit-queue.appspot.com/results/17242359

[Peter Beverloo (cr-android ews)]

Comment on attachment 194790 [details]
Refactoring to prevent further HitTests which disallow ShadowContents

Attachment 194790 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17312203

[Hayato Ito]

Looks like I have to update Source/WebKit, Source/WebKit2 in addition to Source/WebCore.
HitTests are also used there.

[Hayato Ito]

Created attachment 194797 [details]
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews.

[Hayato Ito]

I am wondering how I should test this kind of change, which updated files in a lot of ports.
I've tested the patch on chromium-linux, but looks like it is not enough.
Is there any idea? Can I trust the test coverage of ews?

[EFL EWS Bot]

Comment on attachment 194797 [details]
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews.

Attachment 194797 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17322067

[Peter Beverloo (cr-android ews)]

Comment on attachment 194797 [details]
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews.

Attachment 194797 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17302179

[Dimitri Glazkov (Google)]

(In reply to comment #5)
> Looks like I have to update Source/WebKit, Source/WebKit2 in addition to Source/WebCore.
> HitTests are also used there.

I'd say that if all bubbles are green, we're good to go. This is great, thank you for making this change.

[Hayato Ito]

Created attachment 195024 [details]
Fix a build hopefully

[WebKit Review Bot]

Comment on attachment 195024 [details]
Fix a build hopefully

Clearing flags on attachment: 195024

Committed r146961: <http://trac.webkit.org/changeset/146961>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.