Skip to content

Honour region subtags - #749

Merged
taj-p merged 3 commits into
mainfrom
tajp/honourSubtags
Aug 16, 2026
Merged

Honour region subtags#749
taj-p merged 3 commits into
mainfrom
tajp/honourSubtags

Conversation

@taj-p

@taj-p taj-p commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

LLM Contributions: Regression testing, CI changes (GPT 5.5 Sol).

Fixes #748.

Before After
Subtag not honoured (same glyphs throughout) Subtag honoured
before_subtag after_subtag

Changelog

Fixed

  • Honours script and region sub tags for correct shaping of, for example, zh-Hans and zh-Hant.

@taj-p

taj-p commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

If someone approves, please merge yourself. If you cLGTM with tests removed, I'll remove tests and then merge.

@nicoburns

Copy link
Copy Markdown
Collaborator

I'm personally not a fan of the "create a custom font to test" pattern. If we're going to test this, I'd much rather we find some real fonts to test with. That costs a little repo size, but it makes for a more realistic test and less code to maintain. If we wanted to keep sizes down then we could always subset those fonts.

@taj-p

taj-p commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

I'm personally not a fan of the "create a custom font to test" pattern. If we're going to test this, I'd much rather we find some real fonts to test with. That costs a little repo size, but it makes for a more realistic test and less code to maintain. If we wanted to keep sizes down then we could always subset those fonts.

That's fine by me - preferring realism over non-native speakers being able to easily determine correctness is a choice I am happy with. I'll update the tests accordingly.

@nicoburns nicoburns changed the title Honour subtags Honour language subtags Aug 16, 2026
@taj-p
taj-p requested a review from nicoburns August 16, 2026 22:19
@nicoburns nicoburns changed the title Honour language subtags Honour region subtags Aug 16, 2026
@taj-p
taj-p enabled auto-merge August 16, 2026 22:22
@taj-p
taj-p added this pull request to the merge queue Aug 16, 2026
Merged via the queue into main with commit e00c702 Aug 16, 2026
24 checks passed
@taj-p
taj-p deleted the tajp/honourSubtags branch August 16, 2026 22:31
@DJMcNab

DJMcNab commented Aug 17, 2026

Copy link
Copy Markdown
Member

Sticking a 15MB font file into the repo is at the very least a surprising choice to make without discussion (e.g. at text office hours). It's not the end of the world, but personally I'm inclined to force push a revert; this is a repository size cost we would otherwise have to pay until the end of time.

This is under the assumption that the current font is not subsetted. If this is subsetted, it seems crazy to me that 15MB is the minimal size, but I don't know if there's some table which can't actually be subsetted in play here.

@petamoriken

Copy link
Copy Markdown

Since Web Platform Tests subsets the Noto Font and uses it for testing, I think it is probably possible to reduce the file size.
https://github.com/web-platform-tests/wpt/tree/0cc6a7e1904a69dae98c86412b15bc81256279f8/fonts/noto/cjk

@taj-p

taj-p commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Sticking a 15MB font file into the repo is at the very least a surprising choice to make without discussion (e.g. at text office hours). It's not the end of the world, but personally I'm inclined to force push a revert; this is a repository size cost we would otherwise have to pay until the end of time.

This is under the assumption that the current font is not subsetted. If this is subsetted, it seems crazy to me that 15MB is the minimal size, but I don't know if there's some table which can't actually be subsetted in play here.

I'm happy if you want to force push a revert and I can add this PR back with a subset. Maybe worth force pushing earlier than later?

@DJMcNab

DJMcNab commented Aug 17, 2026

Copy link
Copy Markdown
Member

Done, this has now been force-push reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shaping drops script/region subtags, so zh-Hans and zh-Hant select the same OpenType langsys

4 participants