LibWeb: Respect scroll position set by script during page load

When setting scroll position during page load we need to consider
whether we actually have a fragment to scroll to. A script may already
have run at that point and may already have set a scroll position.

If there is an actual fragment to scroll to, it is fine to scroll to
that fragment, since it should take precedence. If we don't have a
fragment however, we should not unnecessarily overwrite the scroll
position set by the script back to (0, 0).

Since this problem is caused by a spec bug, I have tested the behavior
in the three major browsers engines. Unfortunately they do not agree
fully with each other. If there is no fragment at all (e.g. `foo.html`),
all browsers will respect the scroll position set by the script. If
there is a fragment (e.g. `foo.html#bar`), all browsers will set the
scroll position to the fragment element and ignore the one set by
script. However, when the fragment is empty (e.g. `foo.html#`), then
Blink and WebKit will set scroll position to the fragment, while Gecko
will set scroll position from script. Since all of this is ad-hoc
behavior anyway, I simply implemented the Blink/WebKit behavior because
of the majority vote for now.

This fixes a regression introduced in 51102254b5.
This commit is contained in:
InvalidUsernameException 2025-03-07 22:04:36 +01:00 committed by Alexander Kalenik
parent 53bf0ef225
commit 0e1eb4d4a7
Notes: github-actions[bot] 2025-03-10 16:15:16 +00:00
3 changed files with 22 additions and 2 deletions

View file

@ -18,6 +18,7 @@
#include <LibWeb/DOM/Comment.h>
#include <LibWeb/DOM/Document.h>
#include <LibWeb/DOM/DocumentType.h>
#include <LibWeb/DOM/Element.h>
#include <LibWeb/DOM/ElementFactory.h>
#include <LibWeb/DOM/Event.h>
#include <LibWeb/DOM/ProcessingInstruction.h>
@ -321,8 +322,14 @@ void HTMLParser::the_end(GC::Ref<DOM::Document> document, GC::Ptr<HTMLParser> pa
(void)document->scripts_to_execute_when_parsing_has_finished().take_first();
}
// FIXME: Spec bug: https://github.com/whatwg/html/issues/10914
document->scroll_to_the_fragment();
// AD-HOC: We need to scroll to the fragment on page load somewhere.
// But a script that ran in step 5 above may have scrolled the page already,
// so only do this if there is an actual fragment to avoid resetting the scroll position unexpectedly.
// Spec bug: https://github.com/whatwg/html/issues/10914
auto indicated_part = document->determine_the_indicated_part();
if (indicated_part.has<DOM::Element*>() && indicated_part.get<DOM::Element*>() != nullptr) {
document->scroll_to_the_fragment();
}
// 6. Queue a global task on the DOM manipulation task source given the Document's relevant global object to run the following substeps:
queue_global_task(HTML::Task::Source::DOMManipulation, *document, GC::create_function(heap, [document] {