Bug 113171

Summary:

Allow ShadowContents in HitTests by default.

Product:

WebKit

Reporter:

Hayato Ito <hayato>

Component:

DOM

Assignee:

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:

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

Hayato Ito

Reported 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.

Attachments
Refactoring to prevent further HitTests which disallow ShadowContents (35.69 KB, patch) 2013-03-24 23:21 PDT, Hayato Ito	no flags	Details Formatted Diff Diff
Update also Source/WebKit and Source/WebKit2. Let me watch the result of ews. (60.77 KB, patch) 2013-03-25 00:59 PDT, Hayato Ito	no flags	Details Formatted Diff Diff
Fix a build hopefully (61.32 KB, patch) 2013-03-26 01:03 PDT, Hayato Ito	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Hayato Ito

Comment 1 2013-03-24 23:21:25 PDT

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

Build Bot

Comment 2 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

Hayato Ito

Comment 3 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

Peter Beverloo (cr-android ews)

Comment 4 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

Hayato Ito

Comment 5 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.

Hayato Ito

Comment 6 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.

Hayato Ito

Comment 7 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?

EFL EWS Bot

Comment 8 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

Peter Beverloo (cr-android ews)

Comment 9 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

Dimitri Glazkov (Google)

Comment 10 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.

Hayato Ito

Comment 11 2013-03-26 01:03:37 PDT

Created attachment 195024 [details] Fix a build hopefully

WebKit Review Bot

Comment 12 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>

WebKit Review Bot

Comment 13 2013-03-26 19:42:38 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.