-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Extract Lang attribute for marked contents #20407
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2307,9 +2307,19 @@ class PartialEvaluator { | |
| return; | ||
| } | ||
| // Other marked content types aren't supported yet. | ||
| let props = null; | ||
| if (args[1] instanceof Dict) { | ||
| const lang = args[1].get("Lang"); | ||
| if (typeof lang === "string") { | ||
| props = Object.create(null); | ||
| props.lang = stringToPDFString(lang); | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+2310
to
+2318
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, why are we adding unused data here (outside of a single unit-test)!? This means adding effectively dead code in the Firefox PDF Viewer, and it'll also be overall (ever so slightly) less efficient since this data first of all needs to be parsed and secondly it needs to be sent to the main-thread. |
||
| args = [ | ||
| args[0].name, | ||
| args[1] instanceof Dict ? args[1].get("MCID") : null, | ||
| props, | ||
| ]; | ||
|
|
||
| break; | ||
|
|
@@ -3523,13 +3533,22 @@ class PartialEvaluator { | |
| if (includeMarkedContent) { | ||
| markedContentData.level++; | ||
|
|
||
| const mcid = args[1] instanceof Dict ? args[1].get("MCID") : null; | ||
| let mcid = null; | ||
| let itemLang = null; | ||
| if (args[1] instanceof Dict) { | ||
| mcid = args[1].get("MCID"); | ||
| const langEntry = args[1].get("Lang"); | ||
| if (typeof langEntry === "string") { | ||
| itemLang = stringToPDFString(langEntry); | ||
| } | ||
| } | ||
| textContent.items.push({ | ||
| type: "beginMarkedContentProps", | ||
| id: Number.isInteger(mcid) | ||
| ? `${self.idFactory.getPageObjId()}_mc${mcid}` | ||
| : null, | ||
| tag: args[0] instanceof Name ? args[0].name : null, | ||
| lang: itemLang, | ||
| }); | ||
| } | ||
| break; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -63,6 +63,8 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #lang = null; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #langStack = []; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #layoutTextParams = null; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #pageHeight = 0; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -127,7 +129,6 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| this.#layoutTextParams = { | ||||||||||||||||||||||||||||||||||||
| div: null, | ||||||||||||||||||||||||||||||||||||
| properties: null, | ||||||||||||||||||||||||||||||||||||
| ctx: null, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| const { pageWidth, pageHeight, pageX, pageY } = viewport.rawDims; | ||||||||||||||||||||||||||||||||||||
| this.#transform = [1, 0, 0, -1, -pageX, pageY + pageHeight]; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -232,7 +233,6 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| const params = { | ||||||||||||||||||||||||||||||||||||
| div: null, | ||||||||||||||||||||||||||||||||||||
| properties: null, | ||||||||||||||||||||||||||||||||||||
| ctx: TextLayer.#getCtx(this.#lang), | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| for (const div of this.#textDivs) { | ||||||||||||||||||||||||||||||||||||
| params.properties = this.#textDivProperties.get(div); | ||||||||||||||||||||||||||||||||||||
|
|
@@ -279,8 +279,6 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| if (this.#disableProcessItems) { | ||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| this.#layoutTextParams.ctx ??= TextLayer.#getCtx(this.#lang); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const textDivs = this.#textDivs, | ||||||||||||||||||||||||||||||||||||
| textContentItemsStr = this.#textContentItemsStr; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -308,8 +306,18 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| if (item.tag === "Artifact") { | ||||||||||||||||||||||||||||||||||||
| this.#container.ariaHidden = true; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| // The lang is inherited from the closest enclosing marked content | ||||||||||||||||||||||||||||||||||||
| // (or the top-level language), so that text measurements rely on the | ||||||||||||||||||||||||||||||||||||
| // same font fallback as the one used by the browser to render it. | ||||||||||||||||||||||||||||||||||||
| this.#langStack.push( | ||||||||||||||||||||||||||||||||||||
| item.lang || this.#langStack.at(-1) || this.#lang | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| if (item.lang) { | ||||||||||||||||||||||||||||||||||||
| this.#container.setAttribute("lang", item.lang); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| parent.append(this.#container); | ||||||||||||||||||||||||||||||||||||
| } else if (item.type === "endMarkedContent") { | ||||||||||||||||||||||||||||||||||||
| this.#langStack.pop(); | ||||||||||||||||||||||||||||||||||||
| this.#container = this.#container.parentNode; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -322,12 +330,14 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| #appendText(geom) { | ||||||||||||||||||||||||||||||||||||
| // Initialize all used properties to keep the caches monomorphic. | ||||||||||||||||||||||||||||||||||||
| const textDiv = document.createElement("span"); | ||||||||||||||||||||||||||||||||||||
| const lang = this.#langStack.at(-1) || this.#lang; | ||||||||||||||||||||||||||||||||||||
| const textDivProperties = { | ||||||||||||||||||||||||||||||||||||
| angle: 0, | ||||||||||||||||||||||||||||||||||||
| canvasWidth: 0, | ||||||||||||||||||||||||||||||||||||
| hasText: geom.str !== "", | ||||||||||||||||||||||||||||||||||||
| hasEOL: geom.hasEOL, | ||||||||||||||||||||||||||||||||||||
| fontSize: 0, | ||||||||||||||||||||||||||||||||||||
| lang, | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| this.#textDivs.push(textDiv); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -346,7 +356,7 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| fontFamily = TextLayer.fontFamilyMap.get(fontFamily) || fontFamily; | ||||||||||||||||||||||||||||||||||||
| const fontHeight = Math.hypot(tx[2], tx[3]); | ||||||||||||||||||||||||||||||||||||
| const fontAscent = | ||||||||||||||||||||||||||||||||||||
| fontHeight * TextLayer.#getAscent(fontFamily, style, this.#lang); | ||||||||||||||||||||||||||||||||||||
| fontHeight * TextLayer.#getAscent(fontFamily, style, lang); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| let left, top; | ||||||||||||||||||||||||||||||||||||
| if (angle === 0) { | ||||||||||||||||||||||||||||||||||||
|
|
@@ -422,12 +432,13 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #layout(params) { | ||||||||||||||||||||||||||||||||||||
| const { div, properties, ctx } = params; | ||||||||||||||||||||||||||||||||||||
| const { div, properties } = params; | ||||||||||||||||||||||||||||||||||||
| const { style } = div; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (properties.canvasWidth !== 0 && properties.hasText) { | ||||||||||||||||||||||||||||||||||||
| const { fontFamily } = style; | ||||||||||||||||||||||||||||||||||||
| const { canvasWidth, fontSize } = properties; | ||||||||||||||||||||||||||||||||||||
| const ctx = TextLayer.#getCtx(properties.lang); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| TextLayer.#ensureCtxFont(ctx, fontSize * this.#scale, fontFamily); | ||||||||||||||||||||||||||||||||||||
| // Only measure the width for multi-char text divs, see `appendText`. | ||||||||||||||||||||||||||||||||||||
|
|
@@ -521,9 +532,17 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| static #getAscent(fontFamily, style, lang) { | ||||||||||||||||||||||||||||||||||||
| const cachedAscent = this.#ascentCache.get(fontFamily); | ||||||||||||||||||||||||||||||||||||
| if (cachedAscent) { | ||||||||||||||||||||||||||||||||||||
| return cachedAscent; | ||||||||||||||||||||||||||||||||||||
| // The font fallback (and thus its metrics) depends on the language, hence | ||||||||||||||||||||||||||||||||||||
| // the ascent is cached per-language. | ||||||||||||||||||||||||||||||||||||
| let langAscentCache = this.#ascentCache.get((lang ||= "")); | ||||||||||||||||||||||||||||||||||||
| if (langAscentCache) { | ||||||||||||||||||||||||||||||||||||
| const cachedAscent = langAscentCache.get(fontFamily); | ||||||||||||||||||||||||||||||||||||
| if (cachedAscent) { | ||||||||||||||||||||||||||||||||||||
| return cachedAscent; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| langAscentCache = new Map(); | ||||||||||||||||||||||||||||||||||||
| this.#ascentCache.set(lang, langAscentCache); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+537
to
+545
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can probably be shortened?
Suggested change
|
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| const ctx = this.#getCtx(lang); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -556,7 +575,7 @@ class TextLayer { | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| this.#ascentCache.set(fontFamily, ratio); | ||||||||||||||||||||||||||||||||||||
| langAscentCache.set(fontFamily, ratio); | ||||||||||||||||||||||||||||||||||||
| return ratio; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -942,3 +942,4 @@ | |
| !issue17333.pdf | ||
| !issue20504.pdf | ||
| !issue20504_skia.pdf | ||
| !marked_content_lang.pdf | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4974,6 +4974,21 @@ have written that much by now. So, here’s to squashing bugs.`); | |||||||||||||
| await loadingTask.destroy(); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it("gets operatorList, with marked content lang", async function () { | ||||||||||||||
| const loadingTask = getDocument( | ||||||||||||||
| buildGetDocumentParams("marked_content_lang.pdf") | ||||||||||||||
| ); | ||||||||||||||
| const pdfDoc = await loadingTask.promise; | ||||||||||||||
| const pdfPage = await pdfDoc.getPage(1); | ||||||||||||||
| const opList = await pdfPage.getOperatorList(); | ||||||||||||||
| expect(opList.fnArray[0]).toEqual(OPS.beginMarkedContentProps); | ||||||||||||||
|
calixteman marked this conversation as resolved.
Comment on lines
+4983
to
+4984
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Improve readability by adding a blank line before the
Suggested change
|
||||||||||||||
| expect(opList.argsArray[0][0]).toEqual("P"); | ||||||||||||||
| expect(opList.argsArray[0][2]?.lang).toEqual("en-US"); | ||||||||||||||
| expect(opList.fnArray[10]).toEqual(OPS.beginMarkedContentProps); | ||||||||||||||
| expect(opList.argsArray[10][0]).toEqual("P"); | ||||||||||||||
| expect(opList.argsArray[10][2]?.lang).toEqual("es-ES"); | ||||||||||||||
| }); | ||||||||||||||
|
Comment on lines
+4989
to
+4990
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test doesn't clean-up after itself.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| it("gets operatorList, with page resources containing corrupt /CCITTFaxDecode data", async function () { | ||||||||||||||
| const loadingTask = getDocument( | ||||||||||||||
| buildGetDocumentParams("poppler-90-0-fuzzed.pdf") | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -251,4 +251,29 @@ describe("textLayer", function () { | |||||||||||||
|
|
||||||||||||||
| await loadingTask.destroy(); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it("handles lang attribute for marked content", async function () { | ||||||||||||||
| if (isNodeJS) { | ||||||||||||||
| pending("document.createElement is not supported in Node.js."); | ||||||||||||||
| } | ||||||||||||||
| const loadingTask = getDocument( | ||||||||||||||
| buildGetDocumentParams("marked_content_lang.pdf") | ||||||||||||||
| ); | ||||||||||||||
| const pdfDocument = await loadingTask.promise; | ||||||||||||||
| const page = await pdfDocument.getPage(1); | ||||||||||||||
|
|
||||||||||||||
| const container = document.createElement("div"); | ||||||||||||||
| const textLayer = new TextLayer({ | ||||||||||||||
| textContentSource: page.streamTextContent({ | ||||||||||||||
| includeMarkedContent: true, | ||||||||||||||
| }), | ||||||||||||||
| container, | ||||||||||||||
| viewport: page.getViewport({ scale: 1 }), | ||||||||||||||
| }); | ||||||||||||||
| await textLayer.render(); | ||||||||||||||
|
|
||||||||||||||
| const span = container.querySelector("#p17R_mc1"); | ||||||||||||||
| expect(span.getAttribute("lang")).toEqual("es-ES"); | ||||||||||||||
| expect(span.textContent).toEqual("Esto es español"); | ||||||||||||||
| }); | ||||||||||||||
|
Comment on lines
+277
to
+278
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test doesn't clean-up after itself.
Suggested change
|
||||||||||||||
| }); | ||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.