Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify layout of absolutes with static insets #29894

Merged
merged 1 commit into from Jun 21, 2023

Conversation

mrobinson
Copy link
Member

Absolutes with static insets need to be laid out at their ancestor containing blocks, but their position is dependent on their parent's layout. The static layout position is passed up the tree during hoisting and ancestors each add their own offset to the position until it is relative to the containing block that contains the absolute.

This is currently done with a closure and a fairly tricky "tree rank" numbering system that needs to be threaded through the entire layout. This change replaces that system.

Every time a child is laid out we create a positioning context to hold any absolute children (this can be optimized away at a later time). At each of these moments, we call a method to aggregate offsets to the static insets of hoisted absolutes. This makes the logic easier to follow and will also allow implementing this behavior for inline-blocks, which was impossible with the old system.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because it should not change behavior.

Absolutes with static insets need to be laid out at their ancestor
containing blocks, but their position is dependent on their parent's
layout. The static layout position is passed up the tree during hoisting
and ancestors each add their own offset to the position until it is
relative to the containing block that contains the absolute.

This is currently done with a closure and a fairly tricky "tree rank"
numbering system that needs to be threaded through the entire layout.
This change replaces that system.

Every time a child is laid out we create a positioning context to hold
any absolute children (this can be optimized away at a later time). At
each of these moments, we call a method to aggregate offsets to the
static insets of hoisted absolutes. This makes the logic easier to
follow and will also allow implementing this behavior for inline-blocks,
which was impossible with the old system.
@mrobinson
Copy link
Member Author

@bors-servo try=wpt-2020

@bors-servo
Copy link
Contributor

⌛ Trying commit 459a7d2 with merge 63b8f87...

bors-servo added a commit that referenced this pull request Jun 20, 2023
Simplify layout of absolutes with static insets

Absolutes with static insets need to be laid out at their ancestor containing blocks, but their position is dependent on their parent's layout. The static layout position is passed up the tree during hoisting and ancestors each add their own offset to the position until it is relative to the containing block that contains the absolute.

This is currently done with a closure and a fairly tricky "tree rank" numbering system that needs to be threaded through the entire layout. This change replaces that system.

Every time a child is laid out we create a positioning context to hold any absolute children (this can be optimized away at a later time). At each of these moments, we call a method to aggregate offsets to the static insets of hoisted absolutes. This makes the logic easier to follow and will also allow implementing this behavior for inline-blocks, which was impossible with the old system.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because it should not change behavior.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
State: approved= try=True

1 similar comment
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
State: approved= try=True

@mrobinson
Copy link
Member Author

@bors-servo r=Loirooriol

@bors-servo
Copy link
Contributor

📌 Commit 459a7d2 has been approved by Loirooriol

@bors-servo
Copy link
Contributor

⌛ Testing commit 459a7d2 with merge fa7107a...

@github-actions
Copy link

Test results for linux-wpt-layout-2013 from try job (#5330757030):

Flaky unexpected result (14)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/CSS2/linebox/inline-negative-margin-001.html (#23862)
    • FAIL [expected PASS] subtest: [data-expected-height] 1 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">1234 </span></inline-block>
      height expected 10 but got 20
    • FAIL [expected PASS] subtest: [data-expected-height] 2 assert_equals:
      <inline-block data-expected-height="10">123 <span style="margin-left: -8ch">123 </span></inline-block>
      height expected 10 but got 20
    • PASS [expected FAIL] subtest: [data-expected-height] 4
  • OK /html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin_3.html
    • PASS [expected FAIL] subtest: Restoring window.name on cross-origin history traversal
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
    • PASS [expected FAIL] subtest: Navigating to a different document with form submission
  • TIMEOUT [expected OK] /html/browsers/origin/cross-origin-objects/cross-origin-objects.html (#28569)
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: non-ASCII in filename (formdata event)
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html (#29137)
    • FAIL [expected PASS] subtest: document.write in an imported module assert_true: onload must be called expected true got false
  • ERROR /resource-timing/content-type-parsing.html (#29131)
    • FAIL [expected TIMEOUT] subtest: mime-type 16 : text/html;charset=�gbk assert_equals: expected (string) "text/html" but got (undefined) undefined
    • TIMEOUT [expected NOTRUN] subtest: mime-type 17 : text/html;charset= gbk Test timed out
  • OK /resource-timing/status-codes-create-entry.html (#28675)
    • PASS [expected FAIL] subtest: Make sure all status codes are reported
  • TIMEOUT [expected OK] /webmessaging/with-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out
  • OK [expected TIMEOUT] /webmessaging/without-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank
Stable unexpected results that are known to be intermittent (12)
  • OK [expected TIMEOUT] /_mozilla/mozilla/img_placeholder_load.html (#28717)
    • PASS [expected TIMEOUT] subtest: Loading a placeholder image should trigger an error on the img element
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin.window.html (#29049)
    • FAIL [expected PASS] subtest: Same-origin navigation started from unload handler must be ignored assert_equals: expected "?pass" but got "?fail"
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html (#23849)
    • FAIL [expected NOTRUN] subtest: The document for a standalone media file should have one child in the body. assert_equals: expected "image/png" but got "text/html"
  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
    • PASS [expected FAIL] subtest: first argument: absolute url
  • CRASH [expected OK] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • FAIL [expected TIMEOUT] subtest: Element with tabindex should support autofocus assert_equals: expected "SPAN" but got "BODY"
    • PASS [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus
    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus should support autofocus promise_test: Unhandled rejection with value: object "TypeError: host.attachShadow is not a function"
    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus including no focusable descendants should be skipped promise_test: Unhandled rejection with value: object "TypeError: host.attachShadow is not a function"
    • FAIL [expected NOTRUN] subtest: Area element should support autofocus promise_test: Unhandled rejection with value: object "TypeError: w.document.querySelector(...) is null"
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • FAIL [expected PASS] subtest: multipart/form-data: \r\n in value (formdata event) assert_equals: expected "\r\nContent-Disposition: form-data; name="a"\r\n\r\nb\r\nc\r\n--\r\n" but got ""
  • OK /html/semantics/forms/historical.html (#28568)
    • FAIL [expected PASS] subtest: <input name=isindex> should not be supported assert_regexp_match: expected object "/?isindex=x$/" but got "about:blank"
  • OK [expected TIMEOUT] /html/webappapis/scripting/processing-model-2/unhandled-promise-rejections/promise-rejection-events.html (#26371)
    • FAIL [expected TIMEOUT] subtest: delayed handling: delaying handling rejected promise created from createImageBitmap will cause both events to fire assert_array_equals: expected property 0 to be "InvalidStateError" but got "NotSupportedError" (expected array ["InvalidStateError"] got ["NotSupportedError"])
  • TIMEOUT [expected OK] /webaudio/the-audio-api/the-audiocontext-interface/audiocontext-not-fully-active.html (#27664)

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
Approved by: Loirooriol
Pushing fa7107a to master...

@bors-servo bors-servo merged commit fa7107a into servo:master Jun 21, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged / resolved
Development

Successfully merging this pull request may close these issues.

None yet

3 participants