RESOLVED FIXED 203982
Scrolling to fragment shouldn't happen as a part of updating style
https://bugs.webkit.org/show_bug.cgi?id=203982
Summary Scrolling to fragment shouldn't happen as a part of updating style
Ryosuke Niwa
Reported 2019-11-07 16:33:20 PST
Document::resolveStyle sometimes frameView.scrollToFragment when there were pending stylesheets inside FrameView::scrollToAnchor, which is called by FrameLoader::scrollToFragmentWithParentBoundary.
Attachments
WIP1 (6.68 KB, patch)
2019-11-15 21:13 PST, Ryosuke Niwa
no flags
Patch (8.30 KB, patch)
2019-11-21 17:33 PST, Ryosuke Niwa
no flags
Patch (8.29 KB, patch)
2019-11-21 17:57 PST, Ryosuke Niwa
simon.fraser: review+
Ryosuke Niwa
Comment 1 2019-11-15 21:13:47 PST
Simon Fraser (smfr)
Comment 2 2019-11-18 10:38:34 PST
Comment on attachment 383686 [details] WIP1 View in context: https://bugs.webkit.org/attachment.cgi?id=383686&action=review > Source/WebCore/dom/Document.cpp:3476 > + if (m_gotoAnchorNeededAfterStylesheetsLoad) { > + // https://html.spec.whatwg.org/multipage/browsing-the-web.html#try-to-scroll-to-the-fragment > + eventLoop().queueTask(TaskSource::Networking, *this, [this] { > + if (!m_renderView) > + return; > + m_renderView->frameView().scrollToFragment(m_url); > + }); > + } Shouldn't this be in the "run the scrolling steps" part of the event loop?
Ryosuke Niwa
Comment 3 2019-11-18 14:27:49 PST
(In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 383686 [details] > WIP1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383686&action=review > > > Source/WebCore/dom/Document.cpp:3476 > > + if (m_gotoAnchorNeededAfterStylesheetsLoad) { > > + // https://html.spec.whatwg.org/multipage/browsing-the-web.html#try-to-scroll-to-the-fragment > > + eventLoop().queueTask(TaskSource::Networking, *this, [this] { > > + if (!m_renderView) > > + return; > > + m_renderView->frameView().scrollToFragment(m_url); > > + }); > > + } > > Shouldn't this be in the "run the scrolling steps" part of the event loop? Nope: https://html.spec.whatwg.org/multipage/browsing-the-web.html#scroll-to-the-fragment-identifier which triggers: https://drafts.csswg.org/cssom-view/#perform-a-scroll just scrolls it. There is a separate section about what scrolling step does but that doesn't include the actual scrolling: https://drafts.csswg.org/cssom-view/#scrolling-events
Ryosuke Niwa
Comment 4 2019-11-21 17:33:36 PST
Ryosuke Niwa
Comment 5 2019-11-21 17:57:25 PST
Ryosuke Niwa
Comment 6 2019-11-21 21:09:38 PST
Radar WebKit Bug Importer
Comment 7 2019-11-21 21:10:21 PST
Antti Koivisto
Comment 8 2019-11-22 03:43:21 PST
Comment on attachment 384114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384114&action=review > Source/WebCore/dom/Document.cpp:-2006 > - if (m_gotoAnchorNeededAfterStylesheetsLoad && !styleScope().hasPendingSheets()) > - frameView.scrollToFragment(m_url); You should also remove this code from Document::needsStyleRecalc() // Ensure this happens eventually as it is currently in resolveStyle. This can be removed if the code moves. if (m_gotoAnchorNeededAfterStylesheetsLoad && !styleScope().hasPendingSheets()) return true;
Note You need to log in before you can comment on or make changes to this bug.