Skip to content

layout: Correctly repair OutsideMarker::list_item_style#42825

Merged
Loirooriol merged 1 commit intoservo:mainfrom
Loirooriol:marker-repair-parent-style
Feb 25, 2026
Merged

layout: Correctly repair OutsideMarker::list_item_style#42825
Loirooriol merged 1 commit intoservo:mainfrom
Loirooriol:marker-repair-parent-style

Conversation

@Loirooriol
Copy link
Copy Markdown
Contributor

OutsideMarker::repair_style() was setting list_item_style to the style of the marker, not the style of the list item.

This patch sets it to node.parent_style(context) instead, and it fixes ServoThreadSafeLayoutNode::parent_style() to take the pseudo-element chain into account. This requires modifying a bunch of logic to pass the SharedStyleContext around.

Testing: Adding a new test

@Loirooriol Loirooriol added A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 T-linux-wpt Do a try run of the WPT labels Feb 24, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Feb 24, 2026
@github-actions
Copy link
Copy Markdown

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

@Loirooriol Loirooriol force-pushed the marker-repair-parent-style branch 2 times, most recently from 4e16fc7 to 822a403 Compare February 24, 2026 23:13
@Loirooriol Loirooriol added the T-linux-wpt Do a try run of the WPT label Feb 24, 2026
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#58021) with upstreamable changes.

@github-actions
Copy link
Copy Markdown

⚠️ Try run (#22374086485) cancelled.

@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Feb 24, 2026
@github-actions
Copy link
Copy Markdown

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

@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58021).

@github-actions
Copy link
Copy Markdown

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

Flaky unexpected result (28)
  • OK [expected TIMEOUT] /IndexedDB/idbfactory_open.any.worker.html
    • PASS [expected FAIL] subtest: Calling open() with version argument 1.5 should not throw.
    • PASS [expected TIMEOUT] subtest: Calling open() with version argument 9007199254740991 should not throw.
    • PASS [expected TIMEOUT] subtest: Calling open() with version argument undefined should not throw.
  • PASS [expected FAIL] /_mozilla/css/linear_gradients_reverse_a.html
  • OK /_mozilla/css/offset_properties_inline.html (#40543)
    • FAIL [expected PASS] subtest: offsetTop

      assert_equals: offsetTop of #inline-1 should be 0. expected 0 but got -1
      

    • FAIL [expected PASS] subtest: offsetLeft

      assert_equals: offsetLeft of #inline-2 should be 40. expected 40 but got 25
      

  • OK /_mozilla/mozilla/getBoundingClientRect.html (#39668)
    • FAIL [expected PASS] subtest: getBoundingClientRect 1

      assert_equals: expected 62 but got 60.35
      

  • TIMEOUT [expected OK] /_mozilla/mozilla/img_find_non_sibling_map.html
  • CRASH [expected PASS] /_mozilla/shadow-dom/move-element-with-ua-shadow-tree-crash.html (#39473)
  • ERROR [expected CRASH] /_webgl/conformance2/misc/uninitialized-test-2.html (#41656)
  • TIMEOUT [expected OK] /credential-management/credentialscontainer-frame-basics.https.html (#39430)
    • TIMEOUT [expected FAIL] subtest: navigator.credentials should be undefined in documents generated from data: URLs.

      Test timed out
      

  • OK /css/css-fonts/variations/font-weight-matching.html (#38577)
    • FAIL [expected PASS] subtest: Test @font-face matching for weight 470

      assert_approx_equals: @font-face should be mapped to CSSTest Weights 300. expected 90 +/- 2 but got 180
      

  • OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/accumulated-oversized-payload.https.window.html (#41705)
  • ERROR [expected TIMEOUT] /html/browsers/browsing-the-web/history-traversal/pageswap/pageswap-initial-navigation.html (#40387)
  • 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)&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"
      

  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted

      assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])
      

  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/update-the-rendering.html (#24145)
    • TIMEOUT [expected FAIL] subtest: "Flush autofocus candidates" should be happen before a scroll event and animation frame callbacks

      Test timed out
      

  • 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/forms/form-submission-0/jsurl-form-submit.tentative.html (#36489)
    • PASS [expected FAIL] subtest: Verifies that form submissions scheduled inside javascript: urls take precedence over the javascript: url's return value.
  • TIMEOUT [expected OK] /html/user-activation/navigation-state-reset-sameorigin.html
    • TIMEOUT [expected PASS] subtest: Post-navigation state reset.

      Test timed out
      

  • FAIL [expected PASS] /png/apng/fcTL-blend-source-transparent.html (#39520)
  • FAIL [expected PASS] /png/apng/fcTL-dispose-in-region-previous.html (#41410)
  • OK /resource-timing/test_resource_timing.https.html (#25216)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (xmlhttprequest)
  • TIMEOUT /trusted-types/trusted-types-navigation.html?06-10 (#37920)
    • TIMEOUT [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls in report-only mode.

      Test timed out
      

    • NOTRUN [expected TIMEOUT] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in report-only mode.
  • OK /visual-viewport/resize-event-order.html (#41981)
    • PASS [expected FAIL] subtest: Popup: DOMWindow resize fired before VisualViewport.
  • CRASH [expected OK] /webaudio/the-audio-api/the-mediastreamaudiodestinationnode-interface/closed-audiocontext-construction.html
  • OK /webdriver/tests/classic/accept_alert/accept.py
    • FAIL [expected PASS] subtest: test_accept_in_popup_window

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

  • OK /webdriver/tests/classic/take_screenshot/iframe.py
    • ERROR [expected PASS] subtest: test_always_captures_top_browsing_context

      setup error: ConnectionRefusedError: [Errno 111] Connection refused
      

    • ERROR [expected PASS] subtest: test_source_origin[same_origin]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused
      

    • ERROR [expected PASS] subtest: test_source_origin[cross_origin]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused
      

  • OK [expected TIMEOUT] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
    • PASS [expected TIMEOUT] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe
  • ERROR [expected OK] /workers/baseurl/alpha/sharedworker-in-worker.html (#21315)
Stable unexpected results that are known to be intermittent (19)
  • OK /IndexedDB/transaction-scheduling-mixed-scopes.any.html (#42753)
    • PASS [expected FAIL] subtest: Check that scope restrictions on mixed transactions are enforced.
  • 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 #45
    • PASS [expected FAIL] subtest: WebGL test #47
    • PASS [expected FAIL] subtest: WebGL test #49
    • PASS [expected FAIL] subtest: WebGL test #51
    • FAIL [expected PASS] subtest: WebGL test #53

      assert_true: Texture was smaller than the expected size 2x2 expected true got false
      

    • FAIL [expected PASS] subtest: WebGL test #55

      assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
      

    • FAIL [expected PASS] subtest: WebGL test #57

      assert_true: Texture was smaller than the expected size 2x2 expected true got false
      

    • FAIL [expected PASS] subtest: WebGL test #59

      assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
      

    • PASS [expected FAIL] subtest: WebGL test #61
    • PASS [expected FAIL] subtest: WebGL test #63
    • And 6 more unexpected results...
  • OK /beacon/beacon-basic.https.window.html (#41723)
    • PASS [expected FAIL] subtest: Payload size restriction should be accumulated: type = string
  • OK /content-security-policy/frame-ancestors/frame-ancestors-path-ignored.window.html (#36468)
    • PASS [expected FAIL] subtest: A 'frame-ancestors' CSP directive with a URL that includes a path should be ignored.
  • OK /css/css-fonts/generic-family-keywords-001.html (#37467)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(fangsong)
  • OK /css/css-fonts/generic-family-keywords-003.html (#38994)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted sans-serif (drawing text in a canvas)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(nastaliq) (drawing text in a canvas)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted ui-serif (drawing text in a canvas)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted ui-rounded (drawing text in a canvas)
  • OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/multiple-iframes.https.window.html (#35176)
  • OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
    • PASS [expected FAIL] subtest: sec-fetch-mode
  • OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
    • PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy cross-site destination
  • ERROR [expected OK] /focus/focus-event-after-switching-iframes.sub.html (#40368)
  • 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/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected PASS] subtest: opener of discarded auxiliary browsing context

      assert_object_equals: property "get" expected function "function opener() {
          [native code]
      }" got function "function opener() {
          [native code]
      }"
      

  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
    • FAIL [expected TIMEOUT] subtest: <dialog>-contained autofocus element gets focused when the dialog is shown

      assert_equals: expected "DIV" but got "BODY"
      

  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus should support autofocus

      assert_equals: expected Element node <div autofocus=""></div> but got Element node <body></body>
      

    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus including no focusable descendants should be skipped

      assert_equals: expected Element node <input autofocus=""></input> but got Element node <body><div autofocus=""></div><input autofocus=""></body>
      

    • FAIL [expected NOTRUN] subtest: Area element should support autofocus

      promise_test: Unhandled rejection with value: object "TypeError: can't access property "appendChild", w.document.querySelector(...) is null"
      

  • OK /navigation-timing/test-navigation-type-reload.html (#33334)
    • PASS [expected FAIL] subtest: Reload domComplete > Original domComplete
    • PASS [expected FAIL] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart
    • PASS [expected FAIL] subtest: Reload fetchStart > Original fetchStart
    • PASS [expected FAIL] subtest: Reload loadEventEnd > Original loadEventEnd
    • PASS [expected FAIL] subtest: Reload loadEventStart > Original loadEventStart
  • OK /touch-events/single-tap-when-touchend-listener-use-sync-xhr.html (#41175)
    • PASS [expected FAIL] subtest: Click event should be fired when touchend opens synchronous XHR
  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?31-35 (#38034)
    • PASS [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in report-only mode.
    • FAIL [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ a default policy throwing an exception in enforcing mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
      

    • FAIL [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ a default policy throwing an exception in report-only mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
      

    • FAIL [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ a default policy making the URL invalid in enforcing mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
      

@github-actions
Copy link
Copy Markdown

✨ Try run (#22374216628) succeeded.

@Loirooriol Loirooriol marked this pull request as ready for review February 25, 2026 00:27
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 25, 2026
Comment on lines +335 to +340
fn parent_style(&self, context: &SharedStyleContext) -> Arc<ComputedValues> {
if let Some(chain) = self.pseudo_element_chain.without_innermost() {
let mut parent = *self;
parent.pseudo_element_chain = chain;
return parent.style(context);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate, but I don't see a better option.

Comment on lines +434 to +437
Some(match self.secondary {
Some(_) => Self::unnested(primary),
None => Self::default(),
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some(match self.secondary {
Some(_) => Self::unnested(primary),
None => Self::default(),
})
self.secondary.and_then(|| Self::unnested(primary)).unwrap_or_default();

Copy link
Copy Markdown
Contributor Author

@Loirooriol Loirooriol Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work, I will use

        Some(
            self.secondary
                .map_or_else(Self::default, |_| Self::unnested(primary)),
        )

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 25, 2026
`OutsideMarker::repair_style()` was setting `list_item_style` to the
style of the marker, not the style of the list item.

This patch sets it to `node.parent_style(context)` instead, and it fixes
`ServoThreadSafeLayoutNode::parent_style()` to take the pseudo-element
chain into account. This requires modifying a bunch of logic to pass the
`SharedStyleContext` around.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
@Loirooriol Loirooriol force-pushed the marker-repair-parent-style branch from 822a403 to 5b06e5a Compare February 25, 2026 10:52
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 25, 2026
@Loirooriol Loirooriol enabled auto-merge February 25, 2026 10:52
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#58021).

@Loirooriol Loirooriol added this pull request to the merge queue Feb 25, 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 Feb 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@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 Feb 25, 2026
@servo-highfive servo-highfive added the S-tests-failed The changes caused existing tests to fail. label Feb 25, 2026
@Loirooriol Loirooriol added this pull request to the merge queue Feb 25, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Feb 25, 2026
Merged via the queue into servo:main with commit 60c5977 Feb 25, 2026
39 checks passed
@Loirooriol Loirooriol deleted the marker-repair-parent-style branch February 25, 2026 14:58
@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 Feb 25, 2026
shubhamg13 pushed a commit to shubhamg13/servo that referenced this pull request Mar 2, 2026
`OutsideMarker::repair_style()` was setting `list_item_style` to the
style of the marker, not the style of the list item.

This patch sets it to `node.parent_style(context)` instead, and it fixes
`ServoThreadSafeLayoutNode::parent_style()` to take the pseudo-element
chain into account. This requires modifying a bunch of logic to pass the
`SharedStyleContext` around.

Testing: Adding a new test

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 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.

4 participants