WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(8.30 KB, patch)
2019-11-21 17:33 PST
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Patch
(8.29 KB, patch)
2019-11-21 17:57 PST
,
Ryosuke Niwa
simon.fraser
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2019-11-15 21:13:47 PST
Created
attachment 383686
[details]
WIP1
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
Created
attachment 384110
[details]
Patch
Ryosuke Niwa
Comment 5
2019-11-21 17:57:25 PST
Created
attachment 384114
[details]
Patch
Ryosuke Niwa
Comment 6
2019-11-21 21:09:38 PST
Committed
r252761
: <
https://trac.webkit.org/changeset/252761
>
Radar WebKit Bug Importer
Comment 7
2019-11-21 21:10:21 PST
<
rdar://problem/57418860
>
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.
Top of Page
Format For Printing
XML
Clone This Bug