Videos with controls enabled never receieve 'dragstart' events.
Created attachment 238143 [details] Patch
Attachment 238143 [details] did not pass style-queue: ERROR: Source/WebCore/page/DragController.cpp:724: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 238143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238143&action=review > Source/WebCore/page/DragController.cpp:732 > + if (!state.source->isMediaElement() && !state.source->contains(hitTestResult.innerNode())) > // The original node being dragged isn't under the drag origin anymore... maybe it was > // hidden or moved out from under the cursor. Regardless, we don't want to start a drag on > // something that's not actually under the drag origin. > + // FIXME(136836): Investigate whether all elements should use the containsIncludingShadowDOM() path here. > return false; > + > + if (state.source->isMediaElement() && !state.source->containsIncludingShadowDOM(hitTestResult.innerNode())) > + return false; I think this would be cleaner as: if (!state.source->isMediaElement()) { if (!state.source->contains(hitTestResult.innerNode())) ... } else { if (!state.source->containsIncludingShadowDOM(hitTestResult.innerNode())) } Or better: bool includeShadowDOM = state.source->isMediaElement(); bool sourceContainsHitNode = false; if (!includeShadowDOM) sourceContainsHitNode = state.source->contains(hitTestResult.innerNode()); else sourceContainsHitNode = state.source->containsIncludingShadowDOM(hitTestResult.innerNode()); if (! sourceContainsHitNode) return false
<rdar://problem/18187713>
Committed r173631: <http://trac.webkit.org/changeset/173631>
Follow up fix committed r173632: <http://trac.webkit.org/changeset/173632>
(In reply to comment #5) > Committed r173631: <http://trac.webkit.org/changeset/173631> It broke fast/events/mousedown-inside-dragstart-should-not-cause-crash.html on all Apple Mac bots, as the EWS noticed it before landing: http://build.webkit.org/results/Apple%20Mavericks%20Release%20WK1%20%28Tests%29/r173655%20%288592%29/fast/events/mousedown-inside-dragstart-should-not-cause-crash-crash-log.txt
Wow, Node::contains() will return FALSE if `this` is NULL. So previously, `state.source->contains(hitTestResult.innerNode())` would return FALSE if `state.source` NULL. Now that we're calling a virtual method, a null-dereference crash occurs. The fact that this ever passed is entirely accidental. I'll fix this in a follow-up patch.
Reopening to attach new patch.
Created attachment 238182 [details] Follow up patch
Committed r173664: <http://trac.webkit.org/changeset/173664>
Created attachment 238376 [details] Follow up patch
Comment on attachment 238376 [details] Follow up patch Clearing flags on attachment: 238376 Committed r173764: <http://trac.webkit.org/changeset/173764>