Bug 111217 - REGRESSION(r143727): Clicking / selecting inside an <embed> is broken
Summary: REGRESSION(r143727): Clicking / selecting inside an <embed> is broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Allan Sandfeld Jensen
URL: http://www.newpopulargames.com/game_1...
Keywords: HasReduction, Regression
Depends on:
Blocks: 95204
  Show dependency treegraph
 
Reported: 2013-03-01 13:41 PST by Julien Chaffraix
Modified: 2013-03-07 10:37 PST (History)
14 users (show)

See Also:


Attachments
Zip of the test case as it requires 2 files (536 bytes, application/zip)
2013-03-01 13:41 PST, Julien Chaffraix
no flags Details
Patch (6.45 KB, patch)
2013-03-06 09:25 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (7.15 KB, patch)
2013-03-06 09:34 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (12.94 KB, patch)
2013-03-07 02:27 PST, Allan Sandfeld Jensen
jchaffraix: review+
jchaffraix: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>