Bug 113171

Summary: Allow ShadowContents in HitTests by default.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: DOMAssignee: Hayato Ito <hayato>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, buildbot, dglazkov, d-r, eric, esprehn+autocc, fmalita, gyuyoung.kim, mifenton, ojan.autocc, pdr, peter+ews, rakuco, rniwa, rwlbuis, schenney, tonikitoo, webcomponents-bugzilla, WebkitBugTracker, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 112096    
Attachments:
Description Flags
Refactoring to prevent further HitTests which disallow ShadowContents
none
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews.
none
Fix a build hopefully none

Description Hayato Ito 2013-03-24 22:18:14 PDT
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.
Comment 1 Hayato Ito 2013-03-24 23:21:25 PDT
Created attachment 194790 [details]
Refactoring to prevent further HitTests which disallow ShadowContents
Comment 2 Build Bot 2013-03-25 00:17:14 PDT
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
Comment 3 Hayato Ito 2013-03-25 00:21:30 PDT
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
Comment 4 Peter Beverloo (cr-android ews) 2013-03-25 00:24:50 PDT
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
Comment 5 Hayato Ito 2013-03-25 00:28:42 PDT
Looks like I have to update Source/WebKit, Source/WebKit2 in addition to Source/WebCore.
HitTests are also used there.
Comment 6 Hayato Ito 2013-03-25 00:59:49 PDT
Created attachment 194797 [details]
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews.
Comment 7 Hayato Ito 2013-03-25 01:14:08 PDT
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?
Comment 8 EFL EWS Bot 2013-03-25 01:15:24 PDT
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
Comment 9 Peter Beverloo (cr-android ews) 2013-03-25 02:14:21 PDT
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
Comment 10 Dimitri Glazkov (Google) 2013-03-25 10:09:51 PDT
(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.
Comment 11 Hayato Ito 2013-03-26 01:03:37 PDT
Created attachment 195024 [details]
Fix a build hopefully
Comment 12 WebKit Review Bot 2013-03-26 19:42:33 PDT
Comment on attachment 195024 [details]
Fix a build hopefully

Clearing flags on attachment: 195024

Committed r146961: <http://trac.webkit.org/changeset/146961>
Comment 13 WebKit Review Bot 2013-03-26 19:42:38 PDT
All reviewed patches have been landed.  Closing bug.