font: Consider `lang` attribute when shaping by RichardTjokroutomo · Pull Request #43447 · servo/servo

RichardTjokroutomo · 2026-03-19T03:07:56Z

Fonts: Added language field in the struct ShapingOptions so Harfbuzz can also consider language when shaping glyphs.

Testing: Existing WPT. Most recent try run: link

2 new passes, 5 new fails.

Failures:

/css/css-text-decor/text-emphasis-punctuation-2.html should be a false positive since text-emphasis shorthand hasn't been supported on stylo yet.
Not quite sure about this, but /css/css-text/text-spacing-trim/text-spacing-trim-quote-001.html?class=halt,htb&lang=ja tests text-spacing-trim default behavior on JA texts. Since this property is not defined in Stylo, I believe that this property's behavior (including default) hasn't been considered in Servo. So Servo previously passing the test should be a false positive. As a side note, this test also fails on Firefox.
/html/canvas/element/manual/text/canvas.2d.lang.dynamic.html, /html/canvas/element/manual/text/canvas.2d.lang.html, /html/canvas/element/manual/text/canvas.2d.lang.inherit.disconnected.canvas.html fail because canvas' experimental lang attribute hasn't been supported yet.

credits to @mrobinson for figuring out the reason for last 3 failing WPT tests!

Fixes: #41825

codecov-commenter · 2026-03-19T03:12:19Z

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

mrobinson · 2026-03-19T08:30:32Z

Please update the testing field to with your analysis of the WPT differences. Manual testing is not enough for these kind of changes typically.

RichardTjokroutomo · 2026-03-19T09:13:46Z

Please update the testing field to with your analysis of the WPT differences. Manual testing is not enough for these kind of changes typically.

Updated. I've also pushed the updated WPT expectations. PTAL!

mrobinson · 2026-03-19T09:52:42Z

 url = { workspace = true }
 uuid = { workspace = true, features = ["v4"] }
 webrender_api = { workspace = true }
+icu_locid.workspace = true


This should be inserted into the list in alphabetical order and also follow the pattern of other entries:

Suggested change

icu_locid.workspace = true

icu_locid = { workspace = true }

my bad; I ran cargo add without actually checking...

mrobinson · 2026-03-19T09:52:51Z

    /// The Unicode script property of the characters in this run.
    pub script: Script,
+
+    pub language: Language,


This needs rustdoc.

mrobinson · 2026-03-19T09:53:45Z

+        let lang = parent_style.get_font()._x_lang.clone();
+        let lang_identifier = match lang.0.parse() {
+            Ok(parsed_language) => parsed_language,
+            Err(_) => Language::UND,
+        };


Suggested change

let lang = parent_style.get_font()._x_lang.clone();

let lang_identifier = match lang.0.parse() {

Ok(parsed_language) => parsed_language,

Err(_) => Language::UND,

};

let language = parent_style.get_font()._x_lang.0.parse().unwrap_or(Langauge::UND);

mrobinson · 2026-03-19T09:54:05Z

 fonts_traits = { workspace = true }
 html5ever = { workspace = true }
-icu_locid = { workspace = true }
+icu_locid.workspace = true


Please keep this consistent with the other entries. There's no reason to make one different.

mrobinson · 2026-03-19T09:56:02Z

        let mut current_text_run = UnshapedTextRun::default();
+        current_text_run.set_language(language.clone());


Instead of doing this, add a new() method that takes the Language and remove the implementation of Default on UnshapedTextRun.

mrobinson · 2026-03-19T09:56:13Z

 x25519-dalek = { workspace = true }
 xml5ever = { workspace = true }
 xpath = { workspace = true }
+icu_locid.workspace = true


Please see above comments.

mrobinson · 2026-03-19T09:56:25Z

 gstreamer-base = "0.25"
 gstreamer-gl = "0.25"
 gstreamer-gl-egl = "0.25"
-gstreamer-gl-sys = "0.25"


This looks unrelated?

hmm, maybe it's a leftover from earlier commits? I originally used icu_locale, but switched to icu_locid because icu_locale is based off newer icu version.

I think you should probably revert this particular change as it is unrelated. Cargo.toml isn't automatically generated, so this change comes from some change that you made or was made locally in your working directory.

ah? I thought I've re-added it in previous commit.

My bad; should be good now.

RichardTjokroutomo · 2026-03-19T13:29:14Z

Thanks for the review! I've addressed them all, PTAL again.

mrobinson

Looks good with two changes before landing.

mrobinson · 2026-03-19T15:33:12Z

    pub word_spacing: Au,
    /// The Unicode script property of the characters in this run.
    pub script: Script,
+


Extra line here.

Suggested change

mrobinson · 2026-03-19T15:35:51Z

 gstreamer-base = "0.25"
 gstreamer-gl = "0.25"
 gstreamer-gl-egl = "0.25"
-gstreamer-gl-sys = "0.25"


I think you should probably revert this particular change as it is unrelated. Cargo.toml isn't automatically generated, so this change comes from some change that you made or was made locally in your working directory.

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

RichardTjokroutomo · 2026-03-20T00:57:04Z

                        script,
-                        ..Default::default()
+                        string: Default::default(),
+                        language,


I think this is the cause of new failing tests.

Previously, I leave it to default (Language::UND), but after @mrobinson suggested to add new method for UnshapedTextRun, I set the value to language.

I think setting it to language should be the way to go, so I'll need to investigate the new failing tests, sorry about this...

RichardTjokroutomo · 2026-03-20T03:27:43Z

@mrobinson regarding the 3 new failing tests, here's what's known:

Firefox also fails all 3 tests.
All 3 tests use JS to set the language, but the ref test themselves use html attribute. But Servo can set language via JS (elem.lang = ...). So the reason of failure shouldn't be JS-related.
All 3 tests use FontFace API to set the font. But Servo already support FontFace since this PR, so Servo should've chosen the correct font for the test.

I still think that setting the language field in UnshapedTextRun is correct, but I'm not sure why the 3 WPT tests failed.

I'm thinking that for now, I can either set the language to unknown & add a TODO, or update the WPT expectations and open a new issue, because it seems the problem only exists with <canvas> element. What do you say?

mrobinson · 2026-03-20T07:01:03Z

@RichardTjokroutomo It's probably fine to leave the language set to undefined there and look at it later, but maybe it's worth at least looking at how the results differ first, in case they point to some unexpected behavior:

./mach test-wpt -r /html/canvas/element/manual/text/canvas.2d.lang.html --log-html out.html
Then open out.html to see the differences in the actual versus expected results.

RichardTjokroutomo · 2026-03-20T07:57:51Z

@RichardTjokroutomo It's probably fine to leave the language set to undefined there and look at it later, but maybe it's worth at least looking at how the results differ first, in case they point to some unexpected behavior:

./mach test-wpt -r /html/canvas/element/manual/text/canvas.2d.lang.html --log-html out.html

Then open out.html to see the differences in the actual versus expected results.

Thanks, I didn't know we can do this! I've created an issue and update it once this PR lands (since the code in question doesnt exist in main branch yet)

RichardTjokroutomo · 2026-03-20T08:00:12Z

newest try run (just to be certain).

mrobinson · 2026-03-20T08:07:31Z

@RichardTjokroutomo Were you able to understand the reason that the tests are failing or at least upload a couple of the screenshots here?

RichardTjokroutomo · 2026-03-20T08:28:09Z

@mrobinson hmm, no. For example, for /html/canvas/element/manual/text/canvas.2d.lang.html, the expected result is that one will render "fi" ligature for en text when lang=en & another one for tr text when lang=tr. Instead, both render the ligature for Turkish text.

I plan to upload all screenshots in this issue, but I can upload it here. Gimme a while.

RichardTjokroutomo · 2026-03-20T08:47:10Z

screenshots from test results:

/html/canvas/element/manual/text/canvas.2d.lang.dynamic.html

actual:

expected:

/html/canvas/element/manual/text/canvas.2d.lang.html

actual:

expected:

/html/canvas/element/manual/text/canvas.2d.lang.inherit.disconnected.canvas.html

actual:

expected:

mrobinson · 2026-03-20T09:02:24Z

I think the issue here is that there is also an experimental lang attribute on CanvasRenderingContext2D. We do not support this.

RichardTjokroutomo · 2026-03-20T09:09:13Z

I think the issue here is that there is also an experimental lang attribute on CanvasRenderingContext2D. We do not support this.

In that case, let me update the TODO desc

mrobinson · 2026-03-20T09:09:19Z

@RichardTjokroutomo Okay. Please update the PR with the results and also update the Testing field of the PR to mention the canvas failures, explaining why they are now failing.

…experimental `lang` attribute Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

RichardTjokroutomo · 2026-03-22T10:21:39Z

@mrobinson newest try run succeeded. Is there anything else you want to change?

RichardTjokroutomo requested review from Loirooriol, gterzian, mrobinson and nicoburns as code owners March 19, 2026 03:07

RichardTjokroutomo marked this pull request as draft March 19, 2026 03:12

RichardTjokroutomo force-pushed the font-language branch 2 times, most recently from 8332aea to da0893e Compare March 19, 2026 03:41

Consider language when shaping glyphs

a72859d

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

RichardTjokroutomo force-pushed the font-language branch from da0893e to a72859d Compare March 19, 2026 03:41

RichardTjokroutomo marked this pull request as ready for review March 19, 2026 06:02

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 19, 2026

RichardTjokroutomo changed the title ~~Consider language when shaping glyphs~~ Fonts: Consider language when shaping glyphs Mar 19, 2026

mrobinson requested changes Mar 19, 2026

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 19, 2026

RichardTjokroutomo force-pushed the font-language branch from 3e10dab to 395d7a3 Compare March 19, 2026 13:25

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Mar 19, 2026

RichardTjokroutomo requested a review from mrobinson March 19, 2026 13:29

RichardTjokroutomo force-pushed the font-language branch 2 times, most recently from 5644efa to 98aa949 Compare March 19, 2026 13:48

mrobinson approved these changes Mar 19, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 19, 2026

mrobinson changed the title ~~Fonts: Consider language when shaping glyphs~~ font: Consider `lang attribute when shaping Mar 19, 2026

mrobinson changed the title ~~font: Consider `lang attribute when shaping~~ font: Consider lang attribute when shaping Mar 19, 2026

pdate wpt expectations & code tidy-up

b53d3e8

Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 19, 2026

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 Mar 19, 2026

RichardTjokroutomo commented Mar 20, 2026

View reviewed changes

RichardTjokroutomo mentioned this pull request Mar 20, 2026

Specify language when rendering text on canvas #43474

Open

servo-highfive removed the S-tests-failed The changes caused existing tests to fail. label Mar 20, 2026

RichardTjokroutomo force-pushed the font-language branch from a71b737 to bf5a046 Compare March 20, 2026 08:30

RichardTjokroutomo requested a review from mrobinson March 20, 2026 08:56

RichardTjokroutomo force-pushed the font-language branch from bf5a046 to c022292 Compare March 20, 2026 09:17

script: updated more WPT expectations & added a TODO note on canvas' …

bbdd4a5

…experimental `lang` attribute Signed-off-by: Richard Tjokroutomo <richard.tjokro2@gmail.com>

RichardTjokroutomo force-pushed the font-language branch from c022292 to bbdd4a5 Compare March 20, 2026 09:32

mrobinson approved these changes Mar 23, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 23, 2026

mrobinson added this pull request to the merge queue Mar 23, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 23, 2026

Merged via the queue into servo:main with commit afa4b52 Mar 23, 2026
32 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 23, 2026

delan mentioned this pull request Apr 22, 2026

shaping with the lang attribute does not support region subtags #44430

Open

		let mut current_text_run = UnshapedTextRun::default();
		current_text_run.set_language(language.clone());

Uh oh!

Conversation

RichardTjokroutomo commented Mar 19, 2026 • edited by Loirooriol Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Mar 19, 2026

⚠️ JUnit XML file not found

Uh oh!

mrobinson commented Mar 19, 2026

Uh oh!

RichardTjokroutomo commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RichardTjokroutomo commented Mar 19, 2026

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RichardTjokroutomo commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson commented Mar 20, 2026

Uh oh!

RichardTjokroutomo commented Mar 20, 2026

Uh oh!

RichardTjokroutomo commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson commented Mar 20, 2026

Uh oh!

RichardTjokroutomo commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RichardTjokroutomo commented Mar 20, 2026

Uh oh!

mrobinson commented Mar 20, 2026

Uh oh!

RichardTjokroutomo commented Mar 20, 2026

Uh oh!

mrobinson commented Mar 20, 2026

Uh oh!

RichardTjokroutomo commented Mar 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

RichardTjokroutomo commented Mar 19, 2026 •

edited by Loirooriol

Loading

RichardTjokroutomo commented Mar 20, 2026 •

edited

Loading

RichardTjokroutomo commented Mar 20, 2026 •

edited

Loading

RichardTjokroutomo commented Mar 20, 2026 •

edited

Loading