RESOLVED CONFIGURATION CHANGED 97875
EventHandler should not use shadowAncestorNode() 
https://bugs.webkit.org/show_bug.cgi?id=97875
Summary EventHandler should not use shadowAncestorNode()
Shinya Kawanaka
Reported 2012-09-28 00:52:28 PDT
shadowAncestorNode() is used in handleMouseReleaseEvent(), but it seems to cause some error in nested shadow subtree.
Attachments
Patch (2.29 KB, patch)
2012-10-03 21:13 PDT, Takashi Sakamoto 		dglazkov: review-
DetailsFormatted DiffDiff
Testcase (705 bytes, text/html)
2012-10-08 22:13 PDT, Shinya Kawanaka 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Takashi Sakamoto
Comment 1 2012-10-03 21:13:42 PDT
Created attachment 167028 [details] Patch
Daniel Bates
Comment 2 2012-10-04 11:57:43 PDT
Comment on attachment 167028 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167028&action=review > Source/WebCore/ChangeLog:12 > + Since Node::shadowAncestorNode is deprecated, it should not be used. > + So replaced the method with Node::shadowHost and Node::isInShadowTree. > + > + No new tests, because this patch is just for refactoring. > + All existing tests have already covered the EventHandler's behavior. The description for this change differs from the description in the Bugzilla bug report. In particular, the Bugzilla bug description implies that there is some kind of error that is caused by the use of shadowAncestorNode() in EventHandler::handleMouseReleaseEvent(). I take it that the Bugzilla bug description is inaccurate?
Shinya Kawanaka
Comment 3 2012-10-08 18:27:55 PDT
It would be good to have a test for this. When I clicked in selected text in shadow dom, the selection is not released, while selection in light dom is released.
Shinya Kawanaka
Comment 4 2012-10-08 18:29:57 PDT
Since we have implemented event retargeting, it might be OK if we don't use shadowHost() here. I'm not sure though...
Shinya Kawanaka
Comment 5 2012-10-08 22:10:14 PDT
When we add preventDefault in Shadow DOM, we cannot release selection.
Shinya Kawanaka
Comment 6 2012-10-08 22:13:11 PDT
Created attachment 167691 [details] Testcase Select 'foobar' and click the selection.
Shinya Kawanaka
Comment 7 2012-10-08 23:36:00 PDT
Oops, that might be related to nothing...
Shinya Kawanaka
Comment 8 2012-10-08 23:38:38 PDT
(In reply to comment #4) > Since we have implemented event retargeting, it might be OK if we don't use shadowHost() here. I'm not sure though... This will cause an error for input[type="range"], according to http://trac.webkit.org/changeset/122224
Dimitri Glazkov (Google)
Comment 9 2012-12-03 10:38:06 PST
Sakamoto-san, any updates on the patch? It's sitting in the queue, but I haven't seen any responses to the comments.
Dimitri Glazkov (Google)
Comment 10 2012-12-03 10:38:22 PST
Comment on attachment 167028 [details] Patch Needs test?
Ryosuke Niwa
Comment 11 2019-10-04 22:47:15 PDT
shadowAncestorNode no longer exists.
Radar WebKit Bug Importer
Comment 12 2019-10-04 22:48:15 PDT
<rdar://problem/56003995>
Note You need to log in before you can comment on or make changes to this bug.