Fix r254739.
Created attachment 391592 [details] Patch
Comment on attachment 391592 [details] Patch r=me
Land it after ensuring all the EWS bots get green.
Comment on attachment 391592 [details] Patch oops. r=me too
OK, building worked. Landing.
Committed r257285: <https://trac.webkit.org/changeset/257285>
<rdar://problem/59747287>
Comment on attachment 391592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review > Source/WebCore/html/HTMLLinkElement.cpp:305 > + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake!
Comment on attachment 391592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >> Source/WebCore/html/HTMLLinkElement.cpp:305 >> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); > > Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. Filed https://bugs.webkit.org/show_bug.cgi?id=208171
Comment on attachment 391592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >>> Source/WebCore/html/HTMLLinkElement.cpp:305 >>> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); >> >> Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! > > Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. > Filed https://bugs.webkit.org/show_bug.cgi?id=208171 Oh, I forgot that this was the one that inspired this entire exercise!
Comment on attachment 391592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391592&action=review >>>> Source/WebCore/html/HTMLLinkElement.cpp:305 >>>> + auto scopeExit = makeScopeExit([&] { m_isHandlingBeforeLoad = previous; }); >>> >>> Seems like for this we should figure out what behavior was broken by not setting m_isHandlingBeforeLoad to true, and make a test that would detect that mistake! >> >> Yeah, look like we are lacking the test for this case. To me, this looks like HTMLLinkElement is detached and reinserted during emitting before-load event is attached, but need some investigation. >> Filed https://bugs.webkit.org/show_bug.cgi?id=208171 > > Oh, I forgot that this was the one that inspired this entire exercise! The good thing is that, we also found this case in JSC Parser.cpp :). While it is only used for debug assertions, anyway, having this annotation is super useful! Thanks Emilio!