Skip to content

Commit

Permalink
Scroll To Text Fragement re-loads after every dynamic style sheet loa…
Browse files Browse the repository at this point in the history
…d, and fights user scrolls to do so.

https://bugs.webkit.org/show_bug.cgi?id=264195
rdar://112608578

Reviewed by Simon Fraser.

In order to account for style changes adjusting the scroll of the page, after each dynamic style
sheet load, we recall scrollToFragment to set the page at the correct position again.

We need to stop automatically scrolling after the user has scrolled, as user scroll should trump
all URL programatic scrolls. So we reset the flag that causes scrolls to happen when a user scroll is
detected.

* LayoutTests/http/tests/scroll-to-text-fragment/no-scroll-after-stylesheet-load-expected.txt: Added.
* LayoutTests/http/tests/scroll-to-text-fragment/no-scroll-after-stylesheet-load.html: Added.
* LayoutTests/resources/ui-helper.js:
(window.UIHelper.async initiateUserScroll):
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::didRemoveAllPendingStylesheet):
* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::scrollToFragment):
(WebCore::LocalFrameView::setWasScrolledByUser):
* Source/WebCore/page/LocalFrameView.h:

Canonical link: https://commits.webkit.org/272151@main
  • Loading branch information
megangardner committed Dec 16, 2023
1 parent a1e9575 commit 2dd5d4c
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
PASS: Page scrolls to correct Text Fragment.
PASS: Addition of dynamic style element did not adjust scroll.

Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<!DOCTYPE html>
<meta charset="utf-8" />
<title>Scroll to text fragment - ensure that loading a stylesheet doesn't cause the page to re-scroll</title>
<meta name="assert" content="This test checks that loading a stylesheet doesn't cause the page to re-scroll.">

<head>
<script src="/js-test-resources/ui-helper.js"></script>

<script>

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);
});

function adjustStyle() {
const head = document.head || document.getElementsByTagName("head")[0];
const styleElement = document.createElement('style');

styleElement.type = "text/css";
head.appendChild(styleElement);
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);
}

if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}


async function runTest()
{
if (!testRunner.runUIScript)
return;

location.href = "#:~:text=Scroll%20Point";
var output = "";

await UIHelper.ensurePresentationUpdate();

if (window.pageYOffset > 0)
output += "PASS: Page scrolls to correct Text Fragment.";
else
output += "FAIL: Page does not scroll to correct Text Fragment.";
output += '<br>';

var scrollOffset = window.pageYOffset;

await UIHelper.initiateUserScroll();

adjustStyle();

await UIHelper.renderingUpdate();

if (window.pageYOffset != scrollOffset)
output += "PASS: Addition of dynamic style element did not adjust scroll.";
else
output += "FAIL: Addition of dynamic style element causes page to re-scroll. First Scroll: " + scrollOffset + " Second Scroll: " + window.pageYOffset;
output += '<br>';

var target = document.getElementById('target');
target.innerHTML = output;
target.style.paddingTop = "0px";

if (window.testRunner)
testRunner.notifyDone();
}

window.addEventListener('load', () => {
runTest();
}, false);
</script>
</head>

<body>
<div id="target" style="padding-top: 10000px;">

<p>Scroll Point</p>

</div>
</body>
</html>

2 changes: 2 additions & 0 deletions LayoutTests/platform/gtk/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2648,6 +2648,8 @@ fast/canvas/font-family-system-ui-canvas.html [ Skip ]

fast/line-grid [ Skip ]

http/tests/scroll-to-text-fragment/no-scroll-after-stylesheet-load.html [ Skip ]

# Flaky tests Nov2023
webkit.org/b/264680 compositing/transitions/transform-on-large-layer.html [ ImageOnlyFailure Pass ]
webkit.org/b/264680 fast/canvas/offscreen-backing-store-attached.html [ ImageOnlyFailure Pass ]
Expand Down
10 changes: 10 additions & 0 deletions LayoutTests/resources/ui-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,16 @@ window.UIHelper = class UIHelper {
return new Promise(resolve => setTimeout(resolve, ms));
}

static async initiateUserScroll()
{
if (this.isIOSFamily()) {
await UIHelper.scrollTo(0, 10);
}
else {
await UIHelper.statelessMouseWheelScrollAt(10, 10, 0, 10);
}
}

static scrollTo(x, y, unconstrained)
{
if (!this.isWebKit2()) {
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/page/LocalFrameView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4665,6 +4665,10 @@ void LocalFrameView::setWasScrolledByUser(bool wasScrolledByUser)
cancelScheduledScrolls();
if (currentScrollType() == ScrollType::Programmatic)
return;

if (wasScrolledByUser)
m_frame->document()->setGotoAnchorNeededAfterStylesheetsLoad(false);

m_maintainScrollPositionAnchor = nullptr;
if (m_wasScrolledByUser == wasScrolledByUser)
return;
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12261,6 +12261,7 @@ void WebPageProxy::detectDataInAllFrames(OptionSet<WebCore::DataDetectorType> ty
completionHandler({ });
return;
}

sendWithAsyncReply(Messages::WebPage::DetectDataInAllFrames(types), WTFMove(completionHandler));
}

Expand Down

0 comments on commit 2dd5d4c

Please sign in to comment.