Skip to content

script: Move "scroll into view" call out of "focusing steps"#43842

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:move-scroll-into-view
Apr 1, 2026
Merged

script: Move "scroll into view" call out of "focusing steps"#43842
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:move-scroll-into-view

Conversation

@mrobinson

@mrobinson mrobinson commented Apr 1, 2026

Copy link
Copy Markdown
Member

HTMLOrSVGElement#focus specifies the "scroll into view" steps should
be run after focusing. This was happening implicitly as part of the
Document's implementation of the "focusing steps." That behavior is
not in the specification, so this change moves the scrolling call to
where it is specified. Along with making the code match the
specification, this change simplifies it as well.

Testing: This should not modify behavior, so existing tests should suffice.

@mrobinson mrobinson requested a review from gterzian as a code owner April 1, 2026 16:21
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 1, 2026
@mrobinson mrobinson force-pushed the move-scroll-into-view branch from c1ffa3b to 55d57d5 Compare April 1, 2026 16:23
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 1, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue Apr 1, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 1, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 1, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 1, 2026
`HTMLOrSVGElement#focus` specifies the "scroll into view" steps should
be run after focusing. This was happening implicitly as part of the
`Document`'s implementation of the "focusing steps." That behavior is
not in the specification, so this change moves the scrolling call to
where it is specified. Along with making the code match the
specification, this change simplifies the code.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@mrobinson mrobinson force-pushed the move-scroll-into-view branch from 55d57d5 to 4d7bc26 Compare April 1, 2026 20:53
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 1, 2026
@mrobinson mrobinson added the T-linux-wpt Do a try run of the WPT label Apr 1, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Apr 1, 2026
@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

🔨 Triggering try run (#23870353117) for Linux (WPT)

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

Test results for linux-wpt from try job (#23870353117):

Flaky unexpected result (36)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • PASS [expected TIMEOUT] subtest: Fetching a blob URL immediately before revoking it works in &lt;script&gt; tags.
  • CRASH [expected OK] /content-security-policy/meta/sandbox-iframe.html (#43478)
  • FAIL [expected PASS] /css/css-ui/appearance-checkbox-001.html
  • FAIL [expected PASS] /css/css-ui/appearance-searchfield-001.html
  • FAIL [expected PASS] /css/css-ui/webkit-appearance-menulist-button-001.html
  • FAIL [expected PASS] /css/css-ui/webkit-appearance-meter-001.html
  • FAIL [expected PASS] /css/css-ui/webkit-appearance-radio-001.html
  • OK /dom/nodes/moveBefore/iframe-document-preserve.window.html (#43152)
    • PASS [expected FAIL] subtest: moveBefore(): cross-origin iframe is preserved: remove self
  • OK /fetch/metadata/window-open.https.sub.html (#40339)
    • FAIL [expected PASS] subtest: Same-site window, forced, reloaded

      The operation is insecure.
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/005.html (#27062)
    • FAIL [expected PASS] subtest: Link with onclick navigation and href navigation

      assert_equals: expected "href" but got "click"
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin-fragment.html (#20768)
    • PASS [expected FAIL] subtest: Tests that a fragment navigation in the unload handler will not block the initial navigation
  • OK /html/browsers/history/the-history-interface/traverse_the_history_3.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • CRASH [expected OK] /html/browsers/the-window-object/window-open-noopener.html?_top
  • ERROR [expected OK] /html/canvas/offscreen/text/2d.text.measure.getActualBoundingBox.tentative.html (#43710)
  • OK /html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-1.html (#39694)
    • PASS [expected FAIL] subtest: Meta refresh is blocked by the allow-scripts sandbox flag at its creation time, not when refresh comes due
  • OK /html/semantics/embedded-content/media-elements/playing-the-media-resource/loop-from-ended.tentative.html (#33778)
    • FAIL [expected PASS] subtest: play() with loop set to true after playback ended

      this argument is not a finite floating-point value
      

  • TIMEOUT /html/semantics/embedded-content/media-elements/preserves-pitch.html (#40352)
    • PASS [expected TIMEOUT] subtest: Speed-ups should not change the pitch when preservesPitch=true
    • PASS [expected NOTRUN] subtest: Slow-downs should not change the pitch when preservesPitch=true
    • TIMEOUT [expected NOTRUN] subtest: Speed-ups should change the pitch when preservesPitch=false

      Test timed out
      

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
    • TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup

      Test timed out
      

  • OK /html/semantics/scripting-1/the-script-element/execution-timing/077.html (#22139)
    • FAIL [expected PASS] subtest: adding several types of scripts through the DOM and removing some of them confuses scheduler

      assert_array_equals: expected property 1 to be "Script #1 ran" but got "Script #3 ran" (expected array ["Script #2 ran", "Script #1 ran", "Script #3 ran", "Script #4 ran"] got ["Script #2 ran", "Script #3 ran", "Script #4 ran", "Script #1 ran"])
      

  • TIMEOUT [expected OK] /html/user-activation/navigation-state-reset-sameorigin.html
    • TIMEOUT [expected PASS] subtest: Post-navigation state reset.

      Test timed out
      

  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html (#42419)
    • FAIL [expected PASS] subtest: document.write in an imported module

      assert_true: onload must be called expected true got false
      

  • OK /html/webappapis/user-prompts/print-during-unload.html (#35944)
    • FAIL [expected PASS] subtest: print() during unload

      assert_array_equals: expected property 1 to be "destination" but got "error: window.print is not a function" (expected array ["start", "destination"] got ["start", "error: window.print is not a function"])
      

  • CRASH [expected OK] /import-maps/static-integrity.html
  • OK /mixed-content/tentative/autoupgrades/mixed-content-cors.https.sub.html (#41123)
    • PASS [expected FAIL] subtest: Cross-Origin video should get upgraded even if CORS is set
  • FAIL [expected PASS] /png/apng/fcTL-dispose-previous-final.html (#41837)
  • OK /preload/link-header-preload-delay-onload.html (#39622)
    • FAIL [expected PASS] subtest: Makes sure that Link headers preload resources and block window.onload after resource discovery

      assert_true: expected true got false
      

  • TIMEOUT [expected OK] /preload/modulepreload-sri.html (#43354)
    • TIMEOUT [expected PASS] subtest: Script should not be loaded if modulepreload's integrity is invalid

      Test timed out
      

  • CRASH [expected OK] /svg/animations/svgenum-animation-13.html
  • OK /visual-viewport/resize-event-order.html (#41981)
    • PASS [expected FAIL] subtest: Popup: DOMWindow resize fired before VisualViewport.
  • OK /webdriver/tests/classic/accept_alert/accept.py (#43194)
    • FAIL [expected PASS] subtest: test_null_response_value

      AssertionError: no such alert (404): No user prompt is currently active.
      

  • OK /webdriver/tests/classic/delete_all_cookies/delete.py
    • FAIL [expected PASS] subtest: test_null_response_value

      AssertionError: no such window (404): No such window
      

  • OK /webdriver/tests/classic/dismiss_alert/dismiss.py (#39098)
    • FAIL [expected PASS] subtest: test_null_response_value

      AssertionError: no such alert (404): No user prompt is currently active.
      

  • OK /webdriver/tests/classic/fullscreen_window/fullscreen.py
    • ERROR [expected FAIL] subtest: test_no_top_browsing_context

      setup error: webdriver.error.NoSuchElementException: no such element (404)
      

  • OK /webdriver/tests/classic/get_window_handle/get.py
    • ERROR [expected PASS] subtest: test_no_top_browsing_context

      setup error: webdriver.error.NoSuchElementException: no such element (404)
      

  • OK /webdriver/tests/classic/get_window_handles/get.py
    • ERROR [expected PASS] subtest: test_no_top_browsing_context

      setup error: webdriver.error.NoSuchElementException: no such element (404)
      

  • CRASH [expected OK] /websockets/idlharness.any.html
Stable unexpected results that are known to be intermittent (14)
  • FAIL [expected PASS] /_mozilla/mozilla/sslfail.html (#10760)
  • TIMEOUT [expected OK] /_mozilla/mozilla/window_resize_event.html (#36741)
    • TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo

      Test timed out
      

  • OK /_webgl/conformance/textures/misc/texture-upload-size.html (#21770)
    • PASS [expected FAIL] subtest: WebGL test #93
    • PASS [expected FAIL] subtest: WebGL test #95
    • PASS [expected FAIL] subtest: WebGL test #97
    • PASS [expected FAIL] subtest: WebGL test #99
  • OK /css/css-fonts/generic-family-keywords-001.html (#37467)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(fangsong)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(kai)
  • OK /css/css-fonts/generic-family-keywords-003.html (#38994)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(fangsong) (drawing text in a canvas)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • FAIL [expected PASS] subtest: load event does not fire on window.open('about:blank')

      assert_unreached: load should not be fired Reached unreachable code
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • FAIL [expected PASS] subtest: aElement.click() before the load event must NOT replace

      assert_equals: expected "http://web-platform.test:8000/common/blank.html?thereplacement" but got "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/resources/code-injector.html?pipe=sub(none)&amp;code=%0A%20%20%20%20const%20a%20%3D%20document.createElement(%22a%22)%3B%0A%20%20%20%20a.href%20%3D%20%22%2Fcommon%2Fblank.html%3Fthereplacement%22%3B%0A%20%20%20%20document.currentScript.before(a)%3B%0A%20%20%20%20a.click()%3B%0A%20%20"
      

  • TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • TIMEOUT [expected FAIL] subtest: Element with tabindex should support autofocus

      Test timed out
      

    • NOTRUN [expected TIMEOUT] subtest: Non-HTMLElement should not support autofocus
  • OK /navigation-timing/test-navigation-type-reload.html (#33334)
    • PASS [expected FAIL] subtest: Reload domComplete &gt; Original domComplete
    • PASS [expected FAIL] subtest: Reload domInteractive &gt; Original domInteractive
    • PASS [expected FAIL] subtest: Reload fetchStart &gt; Original fetchStart
    • PASS [expected FAIL] subtest: Reload loadEventEnd &gt; Original loadEventEnd
    • PASS [expected FAIL] subtest: Reload loadEventStart &gt; Original loadEventStart
  • TIMEOUT [expected OK] /preload/modulepreload-sri-importmap.html (#43354)
    • TIMEOUT [expected PASS] subtest: Script should not be loaded if modulepreload's integrity is invalid

      Test timed out
      

  • OK [expected CRASH] /resource-timing/render-blocking-status-link.html (#41664)
  • OK /resource-timing/test_resource_timing.https.html (#25216)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (xmlhttprequest)
  • OK /touch-events/single-tap-when-touchend-listener-use-sync-xhr.html (#41175)
    • FAIL [expected PASS] subtest: Click event should be fired when touchend opens synchronous XHR

      assert_equals: expected "touchend@div, mousedown@div, mouseup@div, click@div" but got "touchend@div"
      

  • OK /trusted-types/trusted-types-reporting.html (#43737)
    • PASS [expected FAIL] subtest: Trusted Type violation report: sample for SVGScriptElement href assignment by setAttribute
    • PASS [expected FAIL] subtest: Trusted Type violation report: Worker constructor

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

✨ Try run (#23870353117) succeeded.

@mrobinson mrobinson added this pull request to the merge queue Apr 1, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 1, 2026
Merged via the queue into servo:main with commit a486ed5 Apr 1, 2026
71 checks passed
@mrobinson mrobinson deleted the move-scroll-into-view branch April 1, 2026 22:22
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants