When an event handler is set on a symbol element, then referenced with a use tag, and finally unset, WebKit crashes in removeEventListener.
<rdar://problem/58818176>
Created attachment 391112 [details] Patch
Comment on attachment 391112 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391112&action=review > Source/WebCore/svg/SVGSVGElement.cpp:146 > - if (!nearestViewportElement()) { > + if (!nearestViewportElement() && isConnected()) { Can we also add a new test testing that resize event handler etc... on a newly created SVG svg element won't be added to the window? e.g. you can do something like this: <!DOCTYPE html> <html> <body> <script src="../../resources/js-test.js"></script> <script> description('This tests creating a disconnected SVG element with resize event handler. The event handler should not get dispatched unless the element is connected'); const iframe = document.createElement('iframe'); iframe.style.width = '100px'; iframe.style.height = '100px'; document.body.appendChild(iframe); didFireResize = false; iframe.contentWindow.requestAnimationFrame(() => { const svg = iframe.contentDocument.createElementNS('http://www.w3.org/2000/svg', 'svg'); svg.setAttribute('onresize', 'top.didFireResize = true'); iframe.style.width = '200px'; iframe.contentWindow.requestAnimationFrame(() => { shouldBeFalse('didFireResize'); }); }); </script> </body>
Created attachment 392321 [details] Patch
Comment on attachment 392321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392321&action=review > Source/WebCore/ChangeLog:13 > + Note that Chrome shares the behavior of a symbol element + use element being transformed into a svg element in the shadow DOM, but Firefox > + does not. This means that Chrome's behavior is slightly different than the WebKit behavior here (they apply event handlers to the parent This is not really relevant. What's important is whether a setting event handler attribute on a disconnected SVG element transfers the event handler to window or not. Firefox doesn't whilst Chrome does. > LayoutTests/fast/events/onresize-svg-parent-window.html:11 > + document.body.appendChild(iframe); > + didFireResize = false; This test doesn't work because Firefox doesn't synchronously load iframe. Let's fix that by putting iframe.contentWindow.requestAnimationFrame inside iframe.onload as in: didFireResize = false; iframe.onload = () { iframe.contentWindow.requestAnimationFrame(() => { .. } document.body.appendChild(iframe); > LayoutTests/fast/events/onresize-svg-parent-window.html:14 > + svg.setAttribute('onresize', 'top.didFireResize = true'); Let's also add a check for on error like this: svg.setAttribute('onerror', 'top.didFireOnError = true'); Declare didFireOnError next to didFireResize. > LayoutTests/fast/events/onresize-svg-parent-window.html:17 > + shouldBeFalse('didFireResize'); Assert also that shouldBeFalse('didFireOnError'); > LayoutTests/fast/events/onresize-svg-parent-window.html:18 > + }); Then after this requestAnimationFrame, do something like this: iframe.contentWindow.eval('throw "error"');
Created attachment 392356 [details] Patch
Comment on attachment 392356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392356&action=review > LayoutTests/fast/events/detached-svg-parent-window-events-expected.txt:9 > +TEST COMPLETE Ugh... we need to define jsTestIsAsync = true. > LayoutTests/fast/events/detached-svg-parent-window-events.html:11 > + didFireOnError = false; Do this here: jsTestIsAsync = true > LayoutTests/fast/events/detached-svg-parent-window-events.html:22 > + if (window.testRunner) > + testRunner.notifyDone(); And call finishJSTest instead of this if (~) testRunner.notifyDone().
Sorry, I keep finding new issues :(
Created attachment 392419 [details] Patch
(In reply to Ryosuke Niwa from comment #8) > Sorry, I keep finding new issues :( Hey, I'd rather find them now when they're easy to fix! Thanks for pointing them out.
Comment on attachment 392419 [details] Patch Nice!
Comment on attachment 392419 [details] Patch Clearing flags on attachment: 392419 Committed r257897: <https://trac.webkit.org/changeset/257897>
All reviewed patches have been landed. Closing bug.