layout: Implement a non-recursive version of CSS `quotes` by xiaochengh · Pull Request #34770 · servo/servo

xiaochengh · 2024-12-24T05:25:57Z

This PR partially implements CSS quotes (#34446):

For non-auto quotes value, retrieves the quote data from the property value
For auto value:
- Hooks up the -x-lang internal property to track the computed lang attribute value
- Uses -x-lang value to determine current locale
- Finds out the correct quotes data according to the ICU CLDR data

It also removes the stale and incorrect UA sheet quotes.css that is not used anywhere.

Quote depth calculation is out of the scope of this PR and will be left for future.

Other caveats:

-x-lang doesn't track the browser/OS-default locale
The quotes data is copy-pasted from the Gecko implementation instead of generated automatically

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes are for Implement CSS quotes #34446 (GitHub issue number if applicable)

There are tests for these changes OR
These changes do not require tests because ___

servo-wpt-sync · 2024-12-24T05:29:33Z

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

servo-wpt-sync · 2025-01-13T07:54:32Z

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

servo-wpt-sync · 2025-01-14T08:11:09Z

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

servo-wpt-sync · 2025-01-14T08:18:48Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#49835) title and body.

servo-wpt-sync · 2025-01-14T08:20:16Z

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

servo-wpt-sync · 2025-01-14T09:52:08Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#49835) title and body.

servo-wpt-sync · 2025-01-14T10:00:13Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#49835) title and body.

servo-wpt-sync · 2025-01-14T10:00:47Z

✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#49835) title and body.

servo-wpt-sync · 2025-01-14T10:23:29Z

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

servo-wpt-sync · 2025-01-15T10:20:25Z

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

xiaochengh · 2025-01-15T10:46:01Z

@Loirooriol Could you take a look and help me trigger the WPT try run? Thanks!

github-actions · 2025-01-15T10:49:54Z

🔨 Triggering try run (#12786736957) for Linux WPT

github-actions · 2025-01-15T11:18:01Z

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

Flaky unexpected result (11)

TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
- NOTRUN [expected PASS] subtest: Overall test

OK /css/css-grid/alignment/grid-content-alignment-with-abspos-001.html (#34339)

FAIL [expected PASS] subtest: .grid 1

assert_equals: 
&lt;div class="grid" data-expected-width="800" data-expected-height="600"&gt;
    &lt;div class="a" id="item" data-offset-x="329" data-offset-y="269" data-expected-width="142" data-expected-height="62" style="place-self: center;"&gt;&lt;/div&gt;
  &lt;/div&gt;
offsetLeft expected 329 but got 0

OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Same site
OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
- PASS [expected FAIL] subtest: load event does not fire on window.open('about:blank')
CRASH [expected OK] /html/canvas/offscreen/canvas-host/2d.canvas.host.size.large.html (#34117)
OK /html/interaction/focus/the-autofocus-attribute/document-with-fragment-valid.html (#28259)
- PASS [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with URL fragments should be skipped.

OK /html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-window-open.html (#32596)

FAIL [expected PASS] subtest: Navigating iframe loading='lazy' before it is loaded: location.replace

uncaught exception: Error: assert_equals: expected "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?nav" but got "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?src"

OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
- PASS [expected FAIL] subtest: text/plain: Basic test (formdata event)
OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
- PASS [expected FAIL] subtest: application/x-www-form-urlencoded: backslash in value (formdata event)
- PASS [expected FAIL] subtest: application/x-www-form-urlencoded: lone surrogate in name and value (formdata event)
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"
```
TIMEOUT [expected OK] /webmessaging/without-ports/017.html (#24486)
- TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank
```
Test timed out
```

Stable unexpected results that are known to be intermittent (16)

FAIL [expected PASS] /_mozilla/css/iframe/hide_and_show.html (#15265)
PASS [expected FAIL] /css/compositing/mix-blend-mode/mix-blend-mode-video-sibling.html (#32849)
OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
- PASS [expected FAIL] subtest: Cross-origin navigation started from unload handler must be ignored
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 /html/browsers/browsing-the-web/navigating-across-documents/refresh/same-document-refresh.html (#34597)

FAIL [expected PASS] subtest: Same-Document Referrer from Refresh

assert_equals: original page loads expected "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/refresh/resources/refresh-with-section.sub.html?url=%23section" but got "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/refresh/resources/refresh-with-section.sub.html?url=%23section#section"

OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
- PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
OK [expected ERROR] /html/canvas/element/manual/imagebitmap/createImageBitmap-colorSpaceConversion.html (#34151)
TIMEOUT [expected ERROR] /html/canvas/element/manual/imagebitmap/createImageBitmap-serializable.html (#34120)
ERROR [expected TIMEOUT] /html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html (#34119)
FAIL [expected PASS] /html/canvas/element/manual/text/canvas.2d.disconnected.html (#30063)
CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)

OK [expected CRASH] /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"

CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
OK /navigation-timing/test-navigation-type-reload.html (#33334)
- PASS [expected FAIL] subtest: Reload domComplete > Original domComplete
- PASS [expected FAIL] subtest: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd
- PASS [expected FAIL] subtest: Reload loadEventEnd > Original loadEventEnd
- PASS [expected FAIL] subtest: Reload loadEventStart > Original loadEventStart
OK /resize-observer/change-layout-in-error.html (#32629)
- PASS [expected FAIL] subtest: Changing layout in window error handler should not result in lifecyle loop when resize observer loop limit is reached.
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

Stable unexpected results (8)

FAIL [expected PASS] /css/CSS2/generated-content/content-056.xht
PASS [expected FAIL] /css/CSS2/generated-content/content-156.xht
PASS [expected FAIL] /css/CSS2/generated-content/content-157.xht
PASS [expected FAIL] /css/CSS2/selectors/first-letter-dynamic-001.xht
PASS [expected FAIL] /css/CSS2/selectors/first-letter-dynamic-002.xht
PASS [expected FAIL] /css/css-content/quotes-016.html
PASS [expected FAIL] /css/css-content/quotes-026.html
FAIL [expected PASS] /css/css-content/quotes-034.html

github-actions · 2025-01-15T11:18:10Z

⚠️ Try run (#12786736957) failed.

servo-wpt-sync · 2025-02-20T08:46:42Z

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

servo-wpt-sync · 2025-02-21T13:14:16Z

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

xiaochengh · 2025-02-21T14:50:06Z

With noto-cjk installed, all other tests have passed except quotes-034.html, which is still rendering the quote as tofu.
And somehow in the ref file, the tofu is continuously shaped with the letter "j" following it; whereas in the test file they are disconnected... 🥲

Expected screenshot	Actual Screenshot

xiaochengh · 2025-02-22T07:43:07Z

How about marking quotes-034.html as intermittent? And how to do that?

I don't want to mark it as failing, as it might pass in some other environments (like in my local env).

mrobinson · 2025-02-22T09:48:30Z

I don't want to mark it as failing, as it might pass in some other environments (like in my local env).

Unfortunately we have a bunch of tests like that. Marking it as intermittent will hide the failure if it ever starts failing on the CI. Would it be possible to start improving our testing environment/setup to ensure more consistent runs when tests are run locally? That is the strategy that WebKit takes, I think.

xiaochengh · 2025-02-22T15:07:06Z

All right. Let me dig deeper to see why the JP quotes are still tofu after installing Noto CJK.

mrobinson · 2025-02-22T16:48:14Z

All right. Let me dig deeper to see why the JP quotes are still tofu after installing Noto CJK.

Oh! I would also say that if the test only fails locally and works as expected on the CI, it shouldn't block this change. I only meant my comment in the general sense. We have a larger issue with tests failing between different systems.

xiaochengh · 2025-02-23T05:17:24Z

This is not a high priority change. Fixing CI might be more valuable, especially for our team.

Loirooriol · 2025-02-23T23:13:58Z

CI getting different results than locally is not a problem specific to this. Also, wpt.fyi considers the test to fail on Firefox and Safari. In fact https://searchfox.org/mozilla-central/source/testing/web-platform/meta/css/css-content/quotes-034.html.ini only expects it to fail on linux, hinting that it's dependent on the environment.

So just mark the test as failing for now and leave investigations for later. Or otherwise servo/stylo#110 should be reverted, since there is no point in accepting the property if it's not going to do anything.

xiaochengh · 2025-02-24T05:57:20Z

IIUC, if we mark it fail, some developers will get stable unexpected passes locally, and they may update the expectation. Is that still OK?

Loirooriol · 2025-02-25T01:17:36Z

Yes, that's not great but it's already happening with several other tests. Like when I run flexbox tests locally, I always get one timeout and 3 passes that are unexpected.

xiaochengh · 2025-02-25T04:44:08Z

OK then, I'll mark it failing. Also filed #35644 to track this.

servo-wpt-sync · 2025-02-25T06:02:50Z

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

Signed-off-by: Xiaocheng Hu <[email protected]>

xiaochengh · 2025-02-27T15:23:10Z

@Loirooriol Done. I wish I can have more time coding...

servo-wpt-sync · 2025-02-27T15:25:20Z

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

servo-wpt-sync mentioned this pull request Dec 24, 2024

layout: Implement a non-recursive version of CSS quotes web-platform-tests/wpt#49835

Merged

xiaochengh force-pushed the localized-quotes branch from 7b914fe to 0ee89b2 Compare January 13, 2025 07:51

xiaochengh force-pushed the localized-quotes branch from 0ee89b2 to 81573ed Compare January 14, 2025 08:08

xiaochengh mentioned this pull request Jan 14, 2025

Enable quotes property servo/stylo#110

Merged

xiaochengh commented Jan 14, 2025

View reviewed changes

Comment thread components/layout_2020/quotes.rs

xiaochengh commented Jan 14, 2025

View reviewed changes

Comment thread tests/wpt/tests/css/css-content/quotes-slot-scoping-ref.html

xiaochengh changed the title ~~WIP implement localized quotes~~ Implement localized CSS quotes Jan 14, 2025

xiaochengh marked this pull request as ready for review January 14, 2025 09:59

xiaochengh requested review from Loirooriol, gterzian, mrobinson and nicoburns as code owners January 14, 2025 09:59

xiaochengh force-pushed the localized-quotes branch from 56be7ef to a2ad25c Compare January 14, 2025 10:20

mrobinson added the T-linux-wpt Do a try run of the WPT label Jan 15, 2025

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Jan 15, 2025

Loirooriol requested changes Jan 15, 2025

View reviewed changes

Comment thread components/layout_2020/dom_traversal.rs Outdated

Comment thread components/layout_2020/dom_traversal.rs Outdated

Comment thread components/layout_2020/quotes.rs

Comment thread components/script/dom/element.rs

xiaochengh mentioned this pull request Feb 21, 2025

Install fonts-noto-cjk for linux-wpt workflow #35567

Merged

5 tasks

xiaochengh force-pushed the localized-quotes branch from 581ae7a to 8f2b263 Compare February 21, 2025 13:11

xiaochengh requested a review from Loirooriol February 25, 2025 06:00

Loirooriol approved these changes Feb 25, 2025

View reviewed changes

Comment thread tests/wpt/meta/css/css-content/inheritance.html.ini Outdated

Comment thread tests/wpt/meta/css/css-content/quotes-slot-scoping.html.ini Outdated

xiaochengh added 3 commits February 27, 2025 23:20

Squash and don't explicitly use noto-cjk in tests

d48d677

Signed-off-by: Xiaocheng Hu <[email protected]>

Mark quotes-034.html.ini failure

72880a9

Signed-off-by: Xiaocheng Hu <[email protected]>

Address review comments

70155c1

Signed-off-by: Xiaocheng Hu <[email protected]>

xiaochengh force-pushed the localized-quotes branch from ba7cd51 to 70155c1 Compare February 27, 2025 15:22

Loirooriol enabled auto-merge February 27, 2025 15:29

Loirooriol added this pull request to the merge queue Feb 27, 2025

Merged via the queue into servo:main with commit 11f54b9 Feb 27, 2025

xiaochengh deleted the localized-quotes branch February 28, 2025 03:42

sakupi01 mentioned this pull request May 13, 2025

layout: Implement quote depth calculation for CSS quotes #36998

Open

5 tasks

servo-wpt-sync mentioned this pull request May 13, 2025

layout: Implement quote depth calculation for CSS quotes web-platform-tests/wpt#52501

Open

Uh oh!

Conversation

xiaochengh commented Dec 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

servo-wpt-sync commented Dec 24, 2024

Uh oh!

servo-wpt-sync commented Jan 13, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

Uh oh!

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 14, 2025

Uh oh!

servo-wpt-sync commented Jan 15, 2025

Uh oh!

xiaochengh commented Jan 15, 2025

Uh oh!

github-actions bot commented Jan 15, 2025

Uh oh!

github-actions bot commented Jan 15, 2025

Uh oh!

github-actions bot commented Jan 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

servo-wpt-sync commented Feb 20, 2025

Uh oh!

servo-wpt-sync commented Feb 21, 2025

Uh oh!

xiaochengh commented Feb 21, 2025

Uh oh!

xiaochengh commented Feb 22, 2025

Uh oh!

mrobinson commented Feb 22, 2025

Uh oh!

xiaochengh commented Feb 22, 2025

Uh oh!

mrobinson commented Feb 22, 2025

Uh oh!

xiaochengh commented Feb 23, 2025

Uh oh!

Loirooriol commented Feb 23, 2025

Uh oh!

xiaochengh commented Feb 24, 2025

Uh oh!

Loirooriol commented Feb 25, 2025

Uh oh!

xiaochengh commented Feb 25, 2025

Uh oh!

servo-wpt-sync commented Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

xiaochengh commented Feb 27, 2025

Uh oh!

servo-wpt-sync commented Feb 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xiaochengh commented Dec 24, 2024 •

edited

Loading