Bug 244895

Summary: Page scrolls to anchor when updating a style element via ECMAScript and a link element was dynamically added on DOMContentReady
Product: WebKit Reporter: Joe Murphy <jmurphy>
Component: ScrollingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: jluther, jmurphy, karlcow, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: BrowserCompat, InRadar
Version: Safari 15   
Hardware: All   
OS: All   
URL: https://jrivera-projects.s3.amazonaws.com/safari-bug/index.html
Attachments:
Description Flags
Testcase that we have to not break when we fix this none

Description Joe Murphy 2022-09-07 09:05:25 PDT 
The page provided (https://jrivera-projects.s3.amazonaws.com/safari-bug/index.html) has code that causes the issue and is interactive. This is a minimal test case but we have seen the problem occurring on several real world sites. One of them is: https://www.pressurecookrecipes.com/instant-pot-mushroom-risotto/#recipe where if you scroll up to the top from the recipe it'll jump back down to the recipe. 
--- 
From the test case linked above: Click on "Go to element #foo" to scroll the page down to the #foo element. Then manually scroll back up here and click on the "Will it jump?" button to see the page scroll back to the #foo element. That scrolling is the issue - the page should NOT scroll. The button here is simply modify a <style> element. There are three seemingly unrelated preconditions that, when present on a page together, trigger this behavior: 

PRECONDITION 1: Anchors The first is the presence of an anchor or "fragment identifier" in the URL. The fragment identifier must reference, by id, one of the elements on the page. 

PRECONDITION 2: Dynamically Inserted Link Tag The second piece of the puzzle is that we need to dynamically add a <link> element to the DOM. In order to trigger this issue, appending of the <link> element must happen either via the "DOMContentLoaded" event or in the "readystatechange" event when document.readyState === 'interactive'. If the <link> is added when document.readyState === 'complete', there will be NO jump. The contents of the file referenced by the href attribute don't matter. As seen in the demo here, the file could even be a 404 and the issue will manifest. Only if the "href" attribute is omitted will the issue not occur. <script type='text/javascript'> // document.addEventListener('DOMContentLoaded', () => { document.addEventListener('readystatechange', () => { if (document.readyState !== 'interactive') { return; } const link = document.createElement('link'); link.rel = 'stylesheet'; link.type = 'text/css'; link.media = 'all'; link.property = 'stylesheet'; link.href = 'not-even-a-file.css'; document.body.appendChild(link); }); </script> 

PRECONDITION 3: Dynamically Updating <Style> Tag For the final part, we simply construct a <style> element with document.createElement('style'); and add some CSS to it as a child "textNode". The actual CSS properties we specify don't seem to matter. I tested with font-size, color, and width but for this demo I leave any style properties empty. The CSS selector also does not need to match any element that is actually in the DOM! Each time the <style> element is updated, the page will jump to the element whose id is in the fragment. <script type='text/javascript'> const head = document.head || document.getElementsByTagName("head")[0]; const styleElement = document.createElement('style'); styleElement.type = "text/css"; head.appendChild(styleElement); const jumpBtn = document.querySelector('#jump'); jumpBtn.addEventListener('click', () => { const css = '#matches-nothing-in-dom {}'; const cssNode = document.createTextNode(css); const childNodes = styleElement.childNodes; const child = childNodes[0]; if (child) { styleElement.replaceChild(cssNode, child); } else { styleElement.appendChild(cssNode); } }); </script>
Comment 1 Joe Murphy 2022-09-07 11:12:43 PDT 
This seems to be affecting various versions of Safari on desktop and mobile as well as Chrome on iOS.
Comment 2 Simon Fraser (smfr) 2022-09-07 21:15:46 PDT 
The unwanted scroll is triggered by `Document::didRemoveAllPendingStylesheet()` in this stack:

    frame #2: 0x00000005e4c545db WebCore`WebCore::FrameView::setScrollPosition(this=0x00000005d20000b0, scrollPosition={ x = 0, y = 8139 }, options=0x00007ff7b8d55638) at FrameView.cpp:2361:21
    frame #3: 0x00000005e4c675bc WebCore`WebCore::FrameView::scrollRectToVisibleInTopLevelView(this=0x00000005d20000b0, absoluteRect={ x = 8px (512), y = 8139.03px (520898), width = 600px (38400), height = 138px (8832) }, insideFixed=false, options=0x00007ff7b8d55860) at FrameView.cpp:2604:9
    frame #4: 0x00000005e4c66f0e WebCore`WebCore::FrameView::scrollRectToVisible(absoluteRect={ x = 8px (512), y = 8139.03px (520898), width = 600px (38400), height = 138px (8832) }, renderer=0x00000005d2011f80, insideFixed=false, options=0x00007ff7b8d55860) at FrameView.cpp:2502:19
    frame #5: 0x00000005e4c66406 WebCore`WebCore::FrameView::scrollToAnchor(this=0x00000005d20000b0) at FrameView.cpp:3465:9
    frame #6: 0x00000005e4c6606b WebCore`WebCore::FrameView::maintainScrollPositionAtAnchor(this=0x00000005d20000b0, anchorNode=0x00000005d2026510) at FrameView.cpp:2326:9
    frame #7: 0x00000005e4c65ca7 WebCore`WebCore::FrameView::scrollToFragmentInternal(this=0x00000005d20000b0, fragmentIdentifier={ length = 3, contents = 'foo' }) at FrameView.cpp:2293:5
    frame #8: 0x00000005e4c6552f WebCore`WebCore::FrameView::scrollToFragment(this=0x00000005d20000b0, url={ https://jrivera-projects.s3.amazonaws.com/safari-bug/index.html#foo }) at FrameView.cpp:2246:9
    frame #9: 0x00000005e3f1d7ff WebCore`WebCore::Document::didRemoveAllPendingStylesheet(this=0x000000059b147f48)::$_11::operator()() const at Document.cpp:3820:24
    frame #10: 0x00000005e3f1d6c9 WebCore`WTF::Detail::CallableWrapper<WebCore::Document::didRemoveAllPendingStylesheet()::$_11, void>::call(this=0x000000059b147f40) at Function.h:53:39
    frame #11: 0x00000005e0a29872 WebCore`WTF::Function<void ()>::operator(this=0x000000059b1296a0)() const at Function.h:82:35
    frame #12: 0x00000005e3fcf329 WebCore`WebCore::EventLoopFunctionDispatchTask::execute(this=0x000000059b129680) at EventLoop.cpp:159:28
    frame #13: 0x00000005e3fc1ad0 WebCore`WebCore::EventLoop::run(this=0x000000059b05ca90) at EventLoop.cpp:123:19
    frame #14: 0x00000005e412d286 WebCore`WebCore::WindowEventLoop::didReachTimeToRun(this=0x000000059b05ca90) at WindowEventLoop.cpp:121:5
    frame #15: 0x00000005e4130169 WebCore`decltype(__f=0x000000059b156d08, __a0=0x000000059b156d18)).*fp()) std::__1::__invoke<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&, void>(void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) at type_traits:3859:1
    frame #16: 0x00000005e41300ed WebCore`std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (__f=0x000000059b156d08, __bound_args=size=1, (null)=__tuple_indices<0> @ 0x00007ff7b8d55fc8, __args=size=0)(), std::__1::tuple<WebCore::WindowEventLoop*>, 0ul, std::__1::tuple<> >(void (WebCore::WindowEventLoop::*&)(), std::__1::tuple<WebCore::WindowEventLoop*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) at bind.h:257:12
    frame #17: 0x00000005e41300a0 WebCore`std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (this=0x000000059b156d08)(), WebCore::WindowEventLoop*>::operator()<>() at bind.h:292:20
    frame #18: 0x00000005e4130039 WebCore`WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*>, void>::call(this=0x000000059b156d00) at Function.h:53:39
    frame #19: 0x00000005e0a29872 WebCore`WTF::Function<void ()>::operator(this=0x000000059b05cb28)() const at Function.h:82:35
    frame #20: 0x00000005e0ad4df9 WebCore`WebCore::Timer::fired(this=0x000000059b05cb00) at Timer.h:135:9
Comment 3 Simon Fraser (smfr) 2022-09-07 21:23:14 PDT 
That task is enqueued by:

    frame #1: 0x000000015c0b6930 WebCore`WebCore::Style::Scope::didRemovePendingStylesheet(this=0x000000010b028b60) at StyleScope.cpp:284:20
    frame #2: 0x000000015c0b6831 WebCore`WebCore::Style::Scope::removePendingSheet(this=0x000000010b028b60, element=0x000000014301fb40) at StyleScope.cpp:242:5
    frame #3: 0x000000015a6660c3 WebCore`WebCore::InlineStyleSheetOwner::sheetLoaded(this=0x000000014301fbc8, element=0x000000014301fb40) at InlineStyleSheetOwner.cpp:238:23
    frame #4: 0x000000015aaafb3f WebCore`WebCore::HTMLStyleElement::sheetLoaded(this=0x000000014301fb40) at HTMLStyleElement.h:64:57
    frame #5: 0x000000015a2c1e77 WebCore`WebCore::StyleSheetContents::checkLoaded(this=0x000000010b033420) at StyleSheetContents.cpp:428:34
    frame #6: 0x000000015a665c77 WebCore`WebCore::InlineStyleSheetOwner::createSheet(this=0x000000014301fbc8, element=0x000000014301fb40, text={ length = 26, contents = '#matches-nothing-in-dom {}' }) at InlineStyleSheetOwner.cpp:209:15
    frame #7: 0x000000015a66543e WebCore`WebCore::InlineStyleSheetOwner::createSheetFromTextContents(this=0x000000014301fbc8, element=0x000000014301fb40) at InlineStyleSheetOwner.cpp:138:5
    frame #8: 0x000000015a6655b9 WebCore`WebCore::InlineStyleSheetOwner::childrenChanged(this=0x000000014301fbc8, element=0x000000014301fb40) at InlineStyleSheetOwner.cpp:126:5
    frame #9: 0x000000015aaad4a4 WebCore`WebCore::HTMLStyleElement::childrenChanged(this=0x000000014301fb40, change=0x00007ff7bb0b3ac8) at HTMLStyleElement.cpp:123:23
    frame #10: 0x000000015a4baf1b WebCore`void WebCore::executeNodeInsertionWithScriptAssertion<WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&)::$_4>(containerNode=0x000000014301fb40, child=0x000000010b3380e0, beforeChild=0x0000000000000000, source=API, replacedAllChildren=No, doNodeInsertion=(unnamed class) @ 0x00007ff7bb0b3b20)::$_4) at ContainerNode.cpp:282:19
    frame #11: 0x000000015a4b8408 WebCore`WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(this=0x000000014301fb40, newChild=0x000000010b3380e0) at ContainerNode.cpp:817:9
    frame #12: 0x000000015a4bada2 WebCore`WebCore::ContainerNode::appendChild(this=0x000000014301fb40, newChild=0x000000010b3380e0) at ContainerNode.cpp:783:12
    frame #13: 0x000000015a6bb7dc WebCore`WebCore::Node::appendChild(this=0x000000014301fb40, newChild=0x000000010b3380e0) at Node.cpp:515:43
    frame #14: 0x000000015840540d WebCore`WebCore::jsNodePrototypeFunction_appendChildBody(this=0x00007ff7bb0b3e68)::'lambda'()::operator()() const at JSNode.cpp:863:102
    frame #15: 0x0000000158405211 WebCore`void WebCore::invokeFunctorPropagatingExceptionIfNecessary<WebCore::jsNodePrototypeFunction_appendChildBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSNode*)::'lambda'()>(lexicalGlobalObject=0x0000000111146068, throwScope=0x00007ff7bb0b3f00, functor=0x00007ff7bb0b3e68)::'lambda'()&&) at JSDOMExceptionHandling.h:96:23
    frame #16: 0x000000015840513d WebCore`WebCore::jsNodePrototypeFunction_appendChildBody(lexicalGlobalObject=0x0000000111146068, callFrame=0x00007ff7bb0b4070, castedThis=0x000000010b2743c8) at JSNode.cpp:863:5

Comments here reference: https://html.spec.whatwg.org/multipage/browsing-the-web.html#try-to-scroll-to-the-fragment
Comment 4 Simon Fraser (smfr) 2022-09-07 21:25:44 PDT 
I think the bug is that m_gotoAnchorNeededAfterStylesheetsLoad should be set to `falase` on the user scroll; it's set to true in FrameLoader::scrollToFragmentWithParentBoundary() when there are pending style sheets, but never cleared.
Comment 5 Joe Murphy 2022-09-14 07:43:04 PDT 
@smfr what is the standard process for getting this bug prioritized?  Please forgive my ignorance, and thanks in advance.
Comment 6 Radar WebKit Bug Importer 2022-09-14 09:06:17 PDT 
<rdar://problem/99921774>
Comment 7 Joe Murphy 2022-09-20 10:43:58 PDT 
webcompat report filed here:
https://webcompat.com/issues/111086
Comment 8 Joe Murphy 2023-01-10 13:13:07 PST 
Hello,
is there any way we can get movement on this one?  This bug is still occurring with hundreds of our customers' sites.  Thank you in advance.
Comment 9 Joe Murphy 2023-09-07 12:03:17 PDT 
Hello again.  We've just passed 1 year since I submitted this bug and I'm desperately trying to get movement here. Can someone help?  This is still an issue and hasn't been resolved with any webkit release in the last year.
Comment 10 Simon Fraser (smfr) 2023-11-02 17:17:16 PDT 
Created attachment 468464 [details]
Testcase that we have to not break when we fix this