Skip to content

Select a single value from XMP language alternatives - #21999

Open
xjtu-ctgg wants to merge 1 commit into
mozilla:masterfrom
xjtu-ctgg:codex/fix-xmp-language-alternatives
Open

xjtu-ctgg wants to merge 1 commit into
mozilla:masterfrom
xjtu-ctgg:codex/fix-xmp-language-alternatives

Conversation

@xjtu-ctgg

@xjtu-ctgg xjtu-ctgg commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #20801.

PDFs with multiple XMP language alternatives currently show concatenated values, such as Hello WorldHello World. For dc:title, dc:description, dc:rights and xmpRights:UsageTerms, select the x-default entry (compared case-insensitively, per the XMP specification), or the first entry when there is no default.

This continues the earlier work in #20874 by @nyxsky404, which the author closed.

Copilot AI lite review requested due to automatic review settings September 21, 2026 14:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread src/core/metadata_parser.js Outdated
const selected =
alternatives.find(node =>
node.attributes.some(
({ name, value }) => name === "xml:lang" && value === "x-default"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the XMP specs:

...all comparisons of xml:lang values shall be case-insensitive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, the x-default comparison is case-insensitive now.

this._parseArray(entry);
continue;
case "dc:title":
case "dc:description":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the XMP specs, dc:rights and xmpRights:UsageTerms has a language alternative too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both.

Comment thread test/unit/metadata_spec.js Outdated
"",
],
[
"<rdf:Bag><rdf:li>One</rdf:li><rdf:li>Two</rdf:li></rdf:Bag>",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's a good idea to test malformed data...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I removed that test.

expect([...metadata]).toEqual([["dc:title", "Foo bar baz"]]);
});

it("should select the default language alternative (issue 20801)", function () {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test and the others, make sure we've a way to know what's failing exactly (in case something is failing) in using withContext.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, each assertion now has a context with the property name and the input.

Comment thread test/unit/metadata_spec.js Outdated
const metadata = createMetadata(data);

expect(metadata.get(name)).toEqual("Hello World");
expect(metadata.getRaw()).toEqual(data);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this check ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No good reason, it came from copying another test. Removed.

Fixes mozilla#20801.

`dc:title`, `dc:description`, `dc:rights` and `xmpRights:UsageTerms` are
language alternatives, so pick the `x-default` entry (compared
case-insensitively, as the XMP specification requires), or the first one
when there is no default, instead of concatenating all of them.

Continues the approach proposed by @nyxsky404 in mozilla#20874.
@xjtu-ctgg
xjtu-ctgg force-pushed the codex/fix-xmp-language-alternatives branch from b70e430 to abb98d9 Compare October 7, 2026 12:38
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Viewer preview

Commit abb98d9 (build logs).

Viewer Build Preview
Modern (gulp generic) ✅ not published
Legacy (gulp generic-legacy) ✅ not published

🔒 A user with write access must approve this commit with the viewer-preview label to publish the viewers.

@codecov-commenter

codecov-commenter commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (d0295c9) to head (abb98d9).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21999      +/-   ##
==========================================
- Coverage   90.22%   90.13%   -0.09%     
==========================================
  Files         275      275              
  Lines       67837    67845       +8     
==========================================
- Hits        61203    61154      -49     
- Misses       6634     6691      +57     
Flag Coverage Δ
browsertest 66.19% <0.00%> (+0.01%) ⬆️
fonttest 8.98% <ø> (ø)
integrationtest 69.20% <100.00%> (-0.82%) ⬇️
unittest 59.91% <100.00%> (-0.02%) ⬇️
unittestcli 58.32% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xjtu-ctgg xjtu-ctgg changed the title Select a single XMP title and description from language alternatives Select a single value from XMP language alternatives Oct 7, 2026

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: PDF title displayed incorrectly when multiple language titles present in XMP metadata

5 participants