e.g. frame #0: JavaScriptCore`WTFCrash+14 frame #1: WebCore`WTFCrashWithInfo(int, char const*, char const*, int)+27 frame #2: WebCore`WebCore::isCloneInShadowTreeOfSVGUseElement(WebCore::Node&, WebCore::EventTarget&)+215 frame #3: WebCore`WebCore::JSLazyEventListener::checkValidityForEventTarget(WebCore::EventTarget&)+215 frame #4: WebCore`WebCore::EventTarget::addEventListener(WTF::AtomString const&, WTF::Ref<WebCore::EventListener, WTF::RawPtrTraits<WebCore::EventListener> >&&, WebCore::AddEventListenerOptions const&)+57 frame #5: WebCore`WebCore::tryAddEventListener(WebCore::Node*, WTF::AtomString const&, WTF::Ref<WebCore::EventListener, WTF::RawPtrTraits<WebCore::EventListener> >&&, WebCore::AddEventListenerOptions const&)+74 frame #6: WebCore`WebCore::Node::addEventListener(WTF::AtomString const&, WTF::Ref<WebCore::EventListener, WTF::RawPtrTraits<WebCore::EventListener> >&&, WebCore::AddEventListenerOptions const&)+69 frame #7: WebCore`WebCore::SVGElement::addEventListener(WTF::AtomString const&, WTF::Ref<WebCore::EventListener, WTF::RawPtrTraits<WebCore::EventListener> >&&, WebCore::AddEventListenerOptions const&)+611 frame #8: WebCore`WebCore::EventTarget::setAttributeEventListener(WTF::AtomString const&, WTF::RefPtr<WebCore::EventListener, WTF::RawPtrTraits<WebCore::EventListener>, WTF::DefaultRefDerefTraits<WebCore::EventListener> >&&, WebCore::DOMWrapperWorld&)+502 frame #9: WebCore`WebCore::Element::setAttributeEventListener(WTF::AtomString const&, WebCore::QualifiedName const&, WTF::AtomString const&)+98 frame #10: WebCore`WebCore::SVGElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomString const&)+393 frame #11: WebCore`WebCore::SVGGraphicsElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomString const&)+169 frame #12: WebCore`WebCore::SVGTextContentElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomString const&)+315 frame #13: WebCore`WebCore::SVGTextPathElement::parseAttribute(WebCore::QualifiedName const&, WTF::AtomString const&)+430 frame #14: WebCore`WebCore::Element::attributeChanged(WebCore::QualifiedName const&, WTF::AtomString const&, WTF::AtomString const&, WebCore::Element::AttributeModificationReason)+1131 frame #15: WebCore`WebCore::StyledElement::attributeChanged(WebCore::QualifiedName const&, WTF::AtomString const&, WTF::AtomString const&, WebCore::Element::AttributeModificationReason)+224 frame #16: WebCore`WebCore::SVGElement::attributeChanged(WebCore::QualifiedName const&, WTF::AtomString const&, WTF::AtomString const&, WebCore::Element::AttributeModificationReason)+58 frame #17: WebCore`WebCore::Element::didAddAttribute(WebCore::QualifiedName const&, WTF::AtomString const&)+68 frame #18: WebCore`WebCore::Element::addAttributeInternal(WebCore::QualifiedName const&, WTF::AtomString const&, WebCore::Element::SynchronizationOfLazyAttribute)+192 frame #19: WebCore`WebCore::Element::setAttributeInternal(unsigned int, WebCore::QualifiedName const&, WTF::AtomString const&, WebCore::Element::SynchronizationOfLazyAttribute)+204 frame #20: WebCore`WebCore::Element::setAttribute(WTF::AtomString const&, WTF::AtomString const&)+389 <rdar://75970108>
There are two problems here. 1. addEventListener is called on a SVG instance that had already been disconnected from use element’s shadow tree but not yet deleted so its correspondingElement hasn’t been cleared yet. 2. DOM mutation events are fired on the corresponding elements of instances inside use element’s shadow tree when there is an EventQueueScope in the stack as in the case of document.execComand. This is because when the instants are cloned and inserted under its parent, isInShadowTree() is false since the nodes had not been inserted into the shadow tree at that moment.
Created attachment 425137 [details] Patch
Created attachment 425138 [details] Fixed change logs
Comment on attachment 425138 [details] Fixed change logs View in context: https://bugs.webkit.org/attachment.cgi?id=425138&action=review > Source/WebCore/dom/ScopedEventQueue.cpp:59 > + if (event.event->eventInterface() == MutationEventInterfaceType && event.target->isInShadowTree()) All shadow trees? The other half of this patch is specific to user agent shadow trees. Do we have tests to verify these events don’t get dispatched for the various types of shadow trees? Should we? Is this just an optimization or an important correctness fix? This seems to have an effect beyond what is mentioned in the change log. > Source/WebCore/svg/SVGElement.cpp:255 > + if (downcast<ShadowRoot>(oldParentOfRemovedTree).mode() == ShadowRootMode::UserAgent) I would have used just one longer if statement instead of two nested if statements. Separately, a helper function named isUserAgenrShadowRoot would make this easier to read and would be consistent with functions like isInUserAgentShadowTree.
(In reply to Darin Adler from comment #4) > Comment on attachment 425138 [details] > Fixed change logs > > View in context: > https://bugs.webkit.org/attachment.cgi?id=425138&action=review > > > Source/WebCore/dom/ScopedEventQueue.cpp:59 > > + if (event.event->eventInterface() == MutationEventInterfaceType && event.target->isInShadowTree()) > > All shadow trees? The other half of this patch is specific to user agent > shadow trees. Do we have tests to verify these events don’t get dispatched > for the various types of shadow trees? Should we? Is this just an > optimization or an important correctness fix? We do for mutation events in general (we don't fire any DOM mutation events inside a shadow tree regardless of its mode). I guess we can add a test for non-UA shadow root as well. > > Source/WebCore/svg/SVGElement.cpp:255 > > + if (downcast<ShadowRoot>(oldParentOfRemovedTree).mode() == ShadowRootMode::UserAgent) > > I would have used just one longer if statement instead of two nested if > statements. > > Separately, a helper function named isUserAgenrShadowRoot would make this > easier to read and would be consistent with functions like > isInUserAgentShadowTree. Good point. Done.
Created attachment 425234 [details] Added a test for regular shadow tree
Created attachment 425235 [details] Added a test for regular shadow tree
Comment on attachment 425235 [details] Added a test for regular shadow tree View in context: https://bugs.webkit.org/attachment.cgi?id=425235&action=review > Source/WebCore/ChangeLog:8 > + The bug was caused by two related but disticnt issues: Typo in comment here: "distinct"
Committed r275543 (236199@main): <https://commits.webkit.org/236199@main>
(In reply to Darin Adler from comment #8) > Comment on attachment 425235 [details] > Added a test for regular shadow tree > > View in context: > https://bugs.webkit.org/attachment.cgi?id=425235&action=review > > > Source/WebCore/ChangeLog:8 > > + The bug was caused by two related but disticnt issues: > > Typo in comment here: "distinct" Fixed.