script: Implement the scrollend event#38773
Merged
yezhizhen merged 3 commits intoservo:mainfrom Mar 20, 2026
Merged
Conversation
mrobinson
changed the title
scrollend event
mrobinson
requested changes
Aug 19, 2025
Comment on lines
+1553
to
+1561
|
|
||
| let scroll_offset = self.scroll_offset(); | ||
| // Step 3 | ||
| let left = x + scroll_offset.x as f64; | ||
| // Step 4 | ||
| let top = y + scroll_offset.y as f64; | ||
|
|
||
| // Step 5 | ||
| self.scroll(left, top, options.parent.behavior); |
Member
There was a problem hiding this comment.
If you are adding more step numbers, please paste the contents of the step text into the comment here as well. You should be able to find other examples as well.
Comment on lines
+133
to
+145
|
|
||
| /// <https://html.spec.whatwg.org/multipage/#dom-focus> | ||
| fn Focus(&self, options: &FocusOptions) { | ||
| let document = self.element.owner_document(); | ||
| document.request_focus_with_options( | ||
| Some(&self.element), | ||
| FocusInitiator::Local, | ||
| FocusOptions { | ||
| preventScroll: options.preventScroll, | ||
| }, | ||
| CanGc::note(), | ||
| ); | ||
| } |
Member
There was a problem hiding this comment.
Can this change be split out into another PR?
Comment on lines
+1007
to
+1009
| if ancestor.is::<Document>() { | ||
| true // Document viewport is always in the containing block chain | ||
| } else if ancestor.is::<Element>() { |
Member
There was a problem hiding this comment.
Please use an early return here instead of a bit nested if statement.
Comment on lines
+1038
to
+1040
| } else { | ||
| false | ||
| } |
Member
There was a problem hiding this comment.
Please use an early return at the top of the function.
Comment on lines
+421
to
+433
| /// <https://html.spec.whatwg.org/multipage/#dom-focus> | ||
| fn Focus(&self, options: &FocusOptions, can_gc: CanGc) { | ||
| // TODO: Mark the element as locked for focus and run the focusing steps. | ||
| // https://html.spec.whatwg.org/multipage/#focusing-steps | ||
| // <https://html.spec.whatwg.org/multipage/#focusing-steps> | ||
| let document = self.owner_document(); | ||
| document.request_focus(Some(self.upcast()), FocusInitiator::Local, can_gc); | ||
| document.request_focus_with_options( | ||
| Some(self.upcast()), | ||
| FocusInitiator::Local, | ||
| FocusOptions { | ||
| preventScroll: options.preventScroll, | ||
| }, | ||
| can_gc, | ||
| ); |
Member
There was a problem hiding this comment.
Can this change to the way that Focus works be moved to a separate PR?
Contributor
Author
|
@mrobinson this PR is rebased on #38754 and #38495, still labeled as draft PR waiting for the other two PRs to be merged first. You can check the implementation of this certain PR in f4522a0 |
abdelrahman1234567
force-pushed
the
scroll_end
branch
2 times, most recently
from
3972feb to
79298d3
Compare
servo-highfive
added
the
S-awaiting-review
Contributor
Author
Member
stevennovaryo
left a comment
stevennovaryo
left a comment
There was a problem hiding this comment.
Looking good! There are several preliminary comments.
I am wondering whether there are more passes from WPTs with testdriver to check the scrollend event from embedder. You can run testdriver WPTs with --product servodriver.
| }; | ||
|
|
||
| document.handle_element_scroll_event(&element); | ||
| document.handle_element_scrollend_event(&element); |
Member
There was a problem hiding this comment.
It should be noted here that we should consider touch panning and trackpad scrolling as well in the future, since the behavior is not exactly the same as instant scroll.
User gestures like touch panning or trackpad scrolling aren’t complete until pointers or keys have released.
Comment on lines
+1651
to
+1658
| /// Whenever scrolling is completed, the user agent must run these steps: | ||
| /// <https://drafts.csswg.org/cssom-view/#scrolling-events> | ||
| /// | ||
| /// Note: This function should be called when all scrolling operations are complete. | ||
| /// For each scrolling box, the corresponding handle_viewport_scrollend_event or | ||
| /// handle_element_scrollend_event should have been called beforehand to populate | ||
| /// the pending_scrollend_event_targets list. | ||
| pub(crate) fn handle_scroll_complete_event(&self, can_gc: CanGc) { |
Member
There was a problem hiding this comment.
There are a recent consensus that we should push all of any pending scroll event (includes scrollend) to a single queue and dispatch them in the next tick. See w3c/csswg-drafts#11164.
The current scrollend event in spec does seems a bit too complicated and not in sync with the scroll event after all.
|
🔨 Triggering try run (#17835071391) for Linux (WPT) |
|
Test results for linux-wpt from try job (#17835071391): Flaky unexpected result (35)
Stable unexpected results that are known to be intermittent (24)
|
|
✨ Try run (#17835071391) succeeded. |
mrobinson
force-pushed
the
scroll_end
branch
2 times, most recently
from
8b10377 to
8ac49a1
Compare
Implementing and handling scrollend even based on spec: https://drafts.csswg.org/cssom-view/#eventdef-document-scrollend Signed-off-by: abdelrahman1234567 <abdelrahman.hossameldin.awadalla@huawei.com>
Member
|
I've revived this one. Sorry for letting it get stale. @stevennovaryo Do you mind taking a look? |
|
🔨 Triggering try run (#23302663998) for Linux (WPT) |
|
|
|
🔨 Triggering try run (#23303098292) for Linux (WPT) |
|
|
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
|
🔨 Triggering try run (#23304137393) for Linux (WPT) |
|
Test results for linux-wpt from try job (#23304137393): Flaky unexpected result (21)
Stable unexpected results that are known to be intermittent (15)
Stable unexpected results (20)
|
|
|
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
|
🔨 Triggering try run (#23338729968) for Linux (WPT) |
|
Test results for linux-wpt from try job (#23338729968): Flaky unexpected result (34)
Stable unexpected results that are known to be intermittent (14)
|
|
✨ Try run (#23338729968) succeeded. |
mrobinson
approved these changes
Mar 20, 2026
servo-highfive
removed
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
servo-highfive
added
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
niyabits
pushed a commit
to niyabits/servo
that referenced
this pull request
Mar 21, 2026
This change implements the `scrollend` event. Since there is no support for asynchronous / smooth scrolling, with this change all `scroll` events simply send a `scrollend` event in a way that looks a bit like what the specification says. Note that there are currently some serious issues (w3c/csswg-drafts#8396) with specification, so things are still a bit weird overall. In addition, this organizes and reduces duplication in some of the existing scroll event code. Testing: This causes some WPT tests to start passing or to stop timing out. --------- Signed-off-by: abdelrahman1234567 <abdelrahman.hossameldin.awadalla@huawei.com> Signed-off-by: Martin Robinson <mrobinson@igalia.com> Co-authored-by: abdelrahman1234567 <abdelrahman.hossameldin.awadalla@huawei.com> Co-authored-by: Martin Robinson <mrobinson@igalia.com> Signed-off-by: Niya Gupta <niyabits@gmail.com>
Gae24
pushed a commit
to Gae24/servo
that referenced
this pull request
Mar 26, 2026
This change implements the `scrollend` event. Since there is no support for asynchronous / smooth scrolling, with this change all `scroll` events simply send a `scrollend` event in a way that looks a bit like what the specification says. Note that there are currently some serious issues (w3c/csswg-drafts#8396) with specification, so things are still a bit weird overall. In addition, this organizes and reduces duplication in some of the existing scroll event code. Testing: This causes some WPT tests to start passing or to stop timing out. --------- Signed-off-by: abdelrahman1234567 <abdelrahman.hossameldin.awadalla@huawei.com> Signed-off-by: Martin Robinson <mrobinson@igalia.com> Co-authored-by: abdelrahman1234567 <abdelrahman.hossameldin.awadalla@huawei.com> Co-authored-by: Martin Robinson <mrobinson@igalia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change implements the
scrollendevent. Since there is no support for asynchronous / smooth scrolling, with this change allscrollevents simply send ascrollendevent in a way that looks a bit like what the specification says. Note that there are currently some serious issues (w3c/csswg-drafts#8396) with specification, so things are still a bit weird overall.In addition, this organizes and reduces duplication in some of the existing scroll event code.
Testing: This causes some WPT tests to start passing or to stop timing out.