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.

Sign up for GitHub

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

Jump to bottom

Support <div align="..."> and <center> in inline layout #31388

Merged
merged 1 commit into from Feb 23, 2024
Merged

Support <div align="..."> and <center> in inline layout #31388

merged 1 commit into from Feb 23, 2024

Conversation

Loirooriol
Copy link
Contributor

As per HTML [1], <div align="..."> and <center> should behave as if they had the text-align property set to the corresponding value.

Servo implements that as internal text-align values because there should the extra effect of aligning block descendants, but that part has not been implemented yet. This patch only adds support for inline layout.

[1]: https://html.spec.whatwg.org/multipage/rendering.html#flow-content-3

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes OR
  • These changes do not require tests because ___

@Loirooriol
Support <div align="..."> and <center> on inline layout
1f975d6 
As per HTML [1], <div align="..."> and <center> should behave as if they
had the text-align property set to the corresponding value.

Servo implements that as internal text-align values because there should
the extra effect of aligning block descendants, but that part has not
been implemented yet. This patch only adds support for inline layout.

[1]: https://html.spec.whatwg.org/multipage/rendering.html#flow-content-3
@Loirooriol Loirooriol added A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 T-linux-wpt-2020 Do a try run of the WPT labels Feb 20, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Feb 20, 2024
@github-actions GitHub Actions
Copy link

🔨 Triggering try run (#7975847475) for Linux WPT

@github-actions GitHub Actions
Copy link

Test results for linux-wpt-layout-2020 from try job (#7975847475):

Flaky unexpected result (12)
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '200 300' over '400'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'oblique 0deg' over 'oblique 10deg 40deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 0deg' should prefer 'oblique 15deg 20deg' over 'oblique 30deg 60deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -21deg' should prefer 'oblique 30deg 60deg' over 'oblique 40deg 50deg'
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • PASS [expected FAIL] subtest: listeners are called correct number of times
  • OK /css/cssom-view/scroll-behavior-smooth-navigation.html (#29564)
    • FAIL [expected PASS] subtest: Smooth scrolling while doing history navigation. assert_not_equals: Shouldn't be scrolled back to top yet. got disallowed value 0
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • FAIL [expected PASS] subtest: 0041 set in href="" targeting a frame and clicked assert_equals: expected "A" but got ""
    • FAIL [expected PASS] subtest: 0080 00FF set in href="" targeting a frame and clicked assert_equals: expected "�ÿ" but got ""
    • FAIL [expected PASS] subtest: 0080 00FF 0100 set in href="" targeting a frame and clicked assert_equals: expected "�ÿĀ" but got ""
  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected TIMEOUT] subtest: opener of discarded auxiliary browsing context assert_equals: opener after removal expected null but got object "[object Window]"
  • 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"
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK /html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html (#23205)
    • PASS [expected FAIL] subtest: Check that rel=noopener with target=_self does a normal load
  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • TIMEOUT [expected FAIL] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document Test timed out
  • TIMEOUT /resource-timing/test_resource_timing.html (#25720)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)
  • OK [expected TIMEOUT] /webmessaging/with-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 (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 PASS] /css/css-fonts/font-size-adjust-reload.html (#30678)
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • TIMEOUT [expected PASS] subtest: background-image sec-fetch-site - HTTPS downgrade (header not sent) Test timed out
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • FAIL [expected PASS] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='' assert_unreached: load should not be fired Reached unreachable code
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • 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/navigation-unload-same-origin.window.html (#29049)
    • PASS [expected FAIL] subtest: Same-origin navigation started from unload handler must be ignored
  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
    • PASS [expected FAIL] subtest: first argument: absolute url
  • TIMEOUT [expected OK] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • 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 ERROR] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • TIMEOUT [expected OK] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry.html (#25805)
    • TIMEOUT [expected FAIL] subtest: Fulfillment handler on pending-then-fulfilled promise Test timed out
    • TIMEOUT [expected FAIL] subtest: Rejection handler on pending-then-rejected promise Test timed out
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out

@github-actions GitHub Actions
Copy link

✨ Try run (#7975847475) succeeded.

@Loirooriol Loirooriol marked this pull request as ready for review February 21, 2024 12:29
@Loirooriol
Copy link
Contributor Author

There is a test about this: html/rendering/non-replaced-elements/flow-content-0/div-align.html

However, it checks the full feature, not just the text alignment, so still failing.

Should I add a more specific test?

@mrobinson mrobinson changed the title Support <div align="..."> and <center> on inline layout Support <div align="..."> and <center> in inline layout Feb 23, 2024
@mrobinson
Copy link
Member

Should I add a more specific test?

I think if tests start passing when the full feature is implemented, it's good enough. Thanks!

@mrobinson mrobinson added this pull request to the merge queue Feb 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 23, 2024
@Loirooriol Loirooriol added this pull request to the merge queue Feb 23, 2024
Merged via the queue into servo:main with commit 38d2ad9 Feb 23, 2024
43 of 44 checks passed
@Loirooriol Loirooriol deleted the support-align-for-inline-layout branch February 23, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson approved these changes

Assignees
No one assigned
Labels
A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Loirooriol @mrobinson