Bug 111217

Summary: REGRESSION(r143727): Clicking / selecting inside an <embed> is broken
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, ap, buildbot, dglazkov, eric, esprehn+autocc, ojan.autocc, peter+ews, philn, rniwa, tonikitoo, webkit-ews, webkit.review.bot, xan.lopez
Priority: P1 Keywords: HasReduction, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.newpopulargames.com/game_1288_Agent-P-Strikes-Back.html
Bug Depends on:    
Bug Blocks: 95204    
Attachments:
Description Flags
Zip of the test case as it requires 2 files
none
Patch
none
Patch
none
Patch jchaffraix: review+, jchaffraix: commit-queue-

Description Julien Chaffraix 2013-03-01 13:41:25 PST 
Created attachment 191028 [details]
Zip of the test case as it requires 2 files

r143727 made us regress the hit testing on embed (haven't tried plugins too). For some reason, we still update the cursor as we move the mouse but the click seems to be stopped at the embed (as shown if you try using the web-inspector).
Comment 1 Alexey Proskuryakov 2013-03-05 10:16:55 PST 
What is the next step here? <http://trac.webkit.org/changeset/143727> sounds like a non-essential improvement per ChangeLog, would rolling it out be appropriate?
Comment 2 Julien Chaffraix 2013-03-05 19:09:55 PST 
Got another reproduction and it turns out that hit-testing through <object> is also broken: http://jsfiddle.net/2RyWt/2/

> What is the next step here? <http://trac.webkit.org/changeset/143727> sounds like a non-essential improvement per ChangeLog, would rolling it out be appropriate?

I am in favor of rolling out unless something is done within 1 or 2 days. I don't have the bandwidth to investigate this bug but I will be happy to handle the revert. The change was supposed to be a refactoring but it turns out to be major hit testing regression.
Comment 3 Allan Sandfeld Jensen 2013-03-06 09:25:16 PST 
Created attachment 191765 [details]
Patch
Comment 4 Early Warning System Bot 2013-03-06 09:29:25 PST 
Comment on attachment 191765 [details]
Patch

Attachment 191765 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16985223
Comment 5 Build Bot 2013-03-06 09:29:53 PST 
Comment on attachment 191765 [details]
Patch

Attachment 191765 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17027081
Comment 6 WebKit Review Bot 2013-03-06 09:30:39 PST 
Comment on attachment 191765 [details]
Patch

Attachment 191765 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16985224
Comment 7 Peter Beverloo (cr-android ews) 2013-03-06 09:34:43 PST 
Comment on attachment 191765 [details]
Patch

Attachment 191765 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17072114
Comment 8 Allan Sandfeld Jensen 2013-03-06 09:34:56 PST 
Created attachment 191770 [details]
Patch

Missed file
Comment 9 Early Warning System Bot 2013-03-06 09:43:41 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17105001
Comment 10 WebKit Review Bot 2013-03-06 09:44:59 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17079151
Comment 11 Build Bot 2013-03-06 09:49:41 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17082237
Comment 12 EFL EWS Bot 2013-03-06 09:49:53 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17049373
Comment 13 WebKit Review Bot 2013-03-06 09:51:02 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17027093
Comment 14 Peter Beverloo (cr-android ews) 2013-03-06 10:01:01 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17072126
Comment 15 Allan Sandfeld Jensen 2013-03-06 10:01:05 PST 
Comment on attachment 191770 [details]
Patch

Needs a test, and be uploaded correctly.
Comment 16 Early Warning System Bot 2013-03-06 10:31:18 PST 
Comment on attachment 191770 [details]
Patch

Attachment 191770 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17027106
Comment 17 Antonio Gomes 2013-03-06 19:28:09 PST 
It should be at least linked to the offending commit.
Comment 18 Allan Sandfeld Jensen 2013-03-07 02:27:16 PST 
Created attachment 191957 [details]
Patch
Comment 19 Julien Chaffraix 2013-03-07 10:12:07 PST 
Comment on attachment 191957 [details]
Patch

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

> LayoutTests/fast/dom/nodesFromRect/nodesFromRect-embedded-frame-content.html:34
> +            <embed id="iframe1" src="resources/child-frame.html"></embed>

There was another test that was linked to that used <object>: http://jsfiddle.net/2RyWt/2/

I think it's worthwhile to add this case too.
Comment 20 Allan Sandfeld Jensen 2013-03-07 10:37:10 PST 
Committed r145098: <http://trac.webkit.org/changeset/145098>