Merge pull request #14074 from Snuffleupagus/issue-14046

[api-minor] Add basic support for RTL text-content in PopupAnnotations (issue 14046)

src/core/annotation.js

@@ -47,6 +47,7 @@ import {
   Name,
   RefSet,
 } from "./primitives.js";
+import { bidi } from "./bidi.js";
 import { Catalog } from "./catalog.js";
 import { ColorSpace } from "./colorspace.js";
 import { FileSpec } from "./file_spec.js";
@@ -356,6 +357,7 @@ class Annotation {
   constructor(params) {
     const dict = params.dict;
 
+    this.setTitle(dict.get("T"));
     this.setContents(dict.get("Contents"));
     this.setModificationDate(dict.get("M"));
     this.setFlags(dict.get("F"));
@@ -374,7 +376,7 @@ class Annotation {
       annotationFlags: this.flags,
       borderStyle: this.borderStyle,
       color: this.color,
-      contents: this.contents,
+      contentsObj: this._contents,
       hasAppearance: !!this.appearance,
       id: params.id,
       modificationDate: this.modificationDate,
@@ -500,17 +502,35 @@ class Annotation {
     return this._isPrintable(this.flags);
   }
 
+  /**
+   * @private
+   */
+  _parseStringHelper(data) {
+    const str = typeof data === "string" ? stringToPDFString(data) : "";
+    const dir = str && bidi(str).dir === "rtl" ? "rtl" : "ltr";
+
+    return { str, dir };
+  }
+
+  /**
+   * Set the title.
+   *
+   * @param {string} title - The title of the annotation, used e.g. with
+   *   PopupAnnotations.
+   */
+  setTitle(title) {
+    this._title = this._parseStringHelper(title);
+  }
+
   /**
    * Set the contents.
    *
-   * @public
-   * @memberof Annotation
    * @param {string} contents - Text to display for the annotation or, if the
    *                            type of annotation does not display text, a
    *                            description of the annotation's contents
    */
   setContents(contents) {
-    this.contents = stringToPDFString(contents || "");
+    this._contents = this._parseStringHelper(contents);
   }
 
   /**
@@ -1014,10 +1034,11 @@ class MarkupAnnotation extends Annotation {
       // the group attributes from the primary annotation.
       const parent = dict.get("IRT");
 
-      this.data.title = stringToPDFString(parent.get("T") || "");
+      this.setTitle(parent.get("T"));
+      this.data.titleObj = this._title;
 
       this.setContents(parent.get("Contents"));
-      this.data.contents = this.contents;
+      this.data.contentsObj = this._contents;
 
       if (!parent.has("CreationDate")) {
         this.data.creationDate = null;
@@ -1043,7 +1064,7 @@ class MarkupAnnotation extends Annotation {
         this.data.color = this.color;
       }
     } else {
-      this.data.title = stringToPDFString(dict.get("T") || "");
+      this.data.titleObj = this._title;
 
       this.setCreationDate(dict.get("CreationDate"));
       this.data.creationDate = this.creationDate;
@@ -2405,8 +2426,11 @@ class PopupAnnotation extends Annotation {
       }
     }
 
-    this.data.title = stringToPDFString(parentItem.get("T") || "");
+    this.setTitle(parentItem.get("T"));
-    this.data.contents = stringToPDFString(parentItem.get("Contents") || "");
+    this.data.titleObj = this._title;
+
+    this.setContents(parentItem.get("Contents"));
+    this.data.contentsObj = this._contents;
   }
 }
 

src/core/bidi.js

@@ -120,7 +120,7 @@ function createBidiText(str, isLTR, vertical = false) {
 const chars = [];
 const types = [];
 
-function bidi(str, startLevel, vertical) {
+function bidi(str, startLevel = -1, vertical = false) {
   let isLTR = true;
   const strLength = str.length;
   if (strLength === 0 || vertical) {

src/display/annotation_layer.js

@@ -320,9 +320,9 @@ class AnnotationElement {
       container,
       trigger,
       color: data.color,
-      title: data.title,
+      titleObj: data.titleObj,
       modificationDate: data.modificationDate,
-      contents: data.contents,
+      contentsObj: data.contentsObj,
       hideWrapper: true,
     });
     const popup = popupElement.render();
@@ -562,8 +562,8 @@ class TextAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable });
   }
@@ -1392,7 +1392,9 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
 
 class PopupAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(parameters.data.title || parameters.data.contents);
+    const isRenderable = !!(
+      parameters.data.titleObj?.str || parameters.data.contentsObj?.str
+    );
     super(parameters, { isRenderable });
   }
 
@@ -1424,9 +1426,9 @@ class PopupAnnotationElement extends AnnotationElement {
       container: this.container,
       trigger: Array.from(parentElements),
       color: this.data.color,
-      title: this.data.title,
+      titleObj: this.data.titleObj,
       modificationDate: this.data.modificationDate,
-      contents: this.data.contents,
+      contentsObj: this.data.contentsObj,
     });
 
     // Position the popup next to the parent annotation's container.
@@ -1456,9 +1458,9 @@ class PopupElement {
     this.container = parameters.container;
     this.trigger = parameters.trigger;
     this.color = parameters.color;
-    this.title = parameters.title;
+    this.titleObj = parameters.titleObj;
     this.modificationDate = parameters.modificationDate;
-    this.contents = parameters.contents;
+    this.contentsObj = parameters.contentsObj;
     this.hideWrapper = parameters.hideWrapper || false;
 
     this.pinned = false;
@@ -1490,7 +1492,8 @@ class PopupElement {
     }
 
     const title = document.createElement("h1");
-    title.textContent = this.title;
+    title.dir = this.titleObj.dir;
+    title.textContent = this.titleObj.str;
     popup.appendChild(title);
 
     // The modification date is shown in the popup instead of the creation
@@ -1508,7 +1511,7 @@ class PopupElement {
       popup.appendChild(modificationDate);
     }
 
-    const contents = this._formatContents(this.contents);
+    const contents = this._formatContents(this.contentsObj);
     popup.appendChild(contents);
 
     if (!Array.isArray(this.trigger)) {
@@ -1531,13 +1534,14 @@ class PopupElement {
    * Format the contents of the popup by adding newlines where necessary.
    *
    * @private
-   * @param {string} contents
+   * @param {Object<string, string>} contentsObj
    * @memberof PopupElement
    * @returns {HTMLParagraphElement}
    */
-  _formatContents(contents) {
+  _formatContents({ str, dir }) {
     const p = document.createElement("p");
-    const lines = contents.split(/(?:\r\n?|\n)/);
+    p.dir = dir;
+    const lines = str.split(/(?:\r\n?|\n)/);
     for (let i = 0, ii = lines.length; i < ii; ++i) {
       const line = lines[i];
       p.appendChild(document.createTextNode(line));
@@ -1601,8 +1605,8 @@ class FreeTextAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -1621,8 +1625,8 @@ class LineAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -1665,8 +1669,8 @@ class SquareAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -1712,8 +1716,8 @@ class CircleAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -1759,8 +1763,8 @@ class PolylineAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
 
@@ -1824,8 +1828,8 @@ class CaretAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -1844,8 +1848,8 @@ class InkAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
 
@@ -1903,8 +1907,8 @@ class HighlightAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, {
       isRenderable,
@@ -1931,8 +1935,8 @@ class UnderlineAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, {
       isRenderable,
@@ -1959,8 +1963,8 @@ class SquigglyAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, {
       isRenderable,
@@ -1987,8 +1991,8 @@ class StrikeOutAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, {
       isRenderable,
@@ -2015,8 +2019,8 @@ class StampAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(
       parameters.data.hasPopup ||
-      parameters.data.title ||
+      parameters.data.titleObj?.str ||
-      parameters.data.contents
+      parameters.data.contentsObj?.str
     );
     super(parameters, { isRenderable, ignoreBorder: true });
   }
@@ -2055,7 +2059,10 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
     trigger.style.width = this.container.style.width;
     trigger.addEventListener("dblclick", this._download.bind(this));
 
-    if (!this.data.hasPopup && (this.data.title || this.data.contents)) {
+    if (
+      !this.data.hasPopup &&
+      (this.data.titleObj?.str || this.data.contentsObj?.str)
+    ) {
       this._createPopup(trigger, this.data);
     }
 

src/display/api.js

@@ -1305,6 +1305,34 @@ class PDFPageProxy {
         intentArgs.renderingIntent
       );
       this._annotationPromises.set(intentArgs.cacheKey, promise);
+
+      if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+        promise = promise.then(annotations => {
+          for (const annotation of annotations) {
+            if (annotation.titleObj !== undefined) {
+              Object.defineProperty(annotation, "title", {
+                get() {
+                  deprecated(
+                    "`title`-property on annotation, please use `titleObj` instead."
+                  );
+                  return annotation.titleObj.str;
+                },
+              });
+            }
+            if (annotation.contentsObj !== undefined) {
+              Object.defineProperty(annotation, "contents", {
+                get() {
+                  deprecated(
+                    "`contents`-property on annotation, please use `contentsObj` instead."
+                  );
+                  return annotation.contentsObj.str;
+                },
+              });
+            }
+          }
+          return annotations;
+        });
+      }
     }
     return promise;
   }

test/pdfs/.gitignore

@@ -15,6 +15,7 @@
 !bug1727053.pdf
 !issue2391-1.pdf
 !issue2391-2.pdf
+!issue14046.pdf
 !issue3214.pdf
 !issue4665.pdf
 !issue4684.pdf

test/test_manifest.json

@@ -5929,5 +5929,12 @@
        "link": true,
        "forms": true,
        "type": "eq"
+    },
+    {  "id": "issue14046",
+       "file": "pdfs/issue14046.pdf",
+       "md5": "dbb5d4e284ca9cc3219e04af6ce64e13",
+       "rounds": 1,
+       "type": "eq",
+       "annotations": true
     }
 ]

test/unit/annotation_spec.js

@@ -312,14 +312,14 @@ describe("annotation", function () {
       const annotation = new Annotation({ dict, ref });
       annotation.setContents("Foo bar baz");
 
-      expect(annotation.contents).toEqual("Foo bar baz");
+      expect(annotation._contents).toEqual({ str: "Foo bar baz", dir: "ltr" });
     });
 
     it("should not set and get invalid contents", function () {
       const annotation = new Annotation({ dict, ref });
       annotation.setContents(undefined);
 
-      expect(annotation.contents).toEqual("");
+      expect(annotation._contents).toEqual({ str: "", dir: "ltr" });
     });
 
     it("should set and get a valid modification date", function () {
@@ -610,8 +610,8 @@ describe("annotation", function () {
       );
       expect(data.inReplyTo).toEqual(annotationRef.toString());
       expect(data.replyType).toEqual("Group");
-      expect(data.title).toEqual("ParentTitle");
+      expect(data.titleObj).toEqual({ str: "ParentTitle", dir: "ltr" });
-      expect(data.contents).toEqual("ParentText");
+      expect(data.contentsObj).toEqual({ str: "ParentText", dir: "ltr" });
       expect(data.creationDate).toEqual("D:20180423");
       expect(data.modificationDate).toEqual("D:20190423");
       expect(data.color).toEqual(new Uint8ClampedArray([0, 0, 255]));
@@ -665,8 +665,8 @@ describe("annotation", function () {
       );
       expect(data.inReplyTo).toEqual(annotationRef.toString());
       expect(data.replyType).toEqual("R");
-      expect(data.title).toEqual("ReplyTitle");
+      expect(data.titleObj).toEqual({ str: "ReplyTitle", dir: "ltr" });
-      expect(data.contents).toEqual("ReplyText");
+      expect(data.contentsObj).toEqual({ str: "ReplyText", dir: "ltr" });
       expect(data.creationDate).toEqual("D:20180523");
       expect(data.modificationDate).toEqual("D:20190523");
       expect(data.color).toEqual(new Uint8ClampedArray([102, 102, 102]));
@@ -3621,8 +3621,8 @@ describe("annotation", function () {
           pdfManagerMock,
           idFactoryMock
         );
-        expect(data.title).toEqual("Correct Title");
+        expect(data.titleObj).toEqual({ str: "Correct Title", dir: "ltr" });
-        expect(data.contents).toEqual("Correct Text");
+        expect(data.contentsObj).toEqual({ str: "Correct Text", dir: "ltr" });
         expect(data.modificationDate).toEqual("D:20190423");
         expect(data.color).toEqual(new Uint8ClampedArray([0, 0, 255]));
       }