pdf viewer renders link annotations incorrectly

[melcher74]

Attach (recommended) or Link to PDF file here: https://cvmg.ca/resources/Au83gabGuDEC94zz/2019/CVMG%20News-5201-Jan%202019.pdf

Configuration:

• Web browser and its version: Firefox Quantum V 64.0
• Operating system and its version: OS X 10.14.2
• PDF.js version: not sure
• Is a browser extension: no

Steps to reproduce the problem:

1. click on the link to view the document in the current Firefox pdf viewer (the problem can only be observed when viewing this specifc file from the link).
2. skip to page 3 of the document to see examples of bookmarks and email links as described below.

What is the expected behavior? (add screenshot) Clickable hotspots (aka link annotations) should be displayed wihtout the border. Viewers should see a light dotted outline, or no outline until hovered around the link.

What went wrong? (add screenshot) In the Firefox pdf viewer, link annotations are displayed with a 1px black border around each. This decreases readability and makes the document display inconsistent with other browsers.

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

[Snuffleupagus]

According to the PDF specification, please see https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#G11.2096210, the W entry of a BS (border style) dictionary should be a number. In this PDF file it's for some inexplicable reason a Name instead, thus making the document corrupt.

I'm really not sure if it's a good solution, but it seems that something along these lines would work:

diff --git a/src/core/annotation.js b/src/core/annotation.js
index cd2dd298..b92bd22b 100644
--- a/src/core/annotation.js
+++ b/src/core/annotation.js
@@ -482,6 +482,9 @@ class AnnotationBorderStyle {
    * @param {integer} width - The width
    */
   setWidth(width) {
+    if (isName(width)) {
+      width = parseFloat(width.name);
+    }
     if (Number.isInteger(width)) {
       this.width = width;
     }
@@ -492,11 +495,11 @@ class AnnotationBorderStyle {
    *
    * @public
    * @memberof AnnotationBorderStyle
-   * @param {Object} style - The style object
+   * @param {Name} style - The style object
    * @see {@link shared/util.js}
    */
   setStyle(style) {
-    if (!style) {
+    if (!isName(style)) { // Unrelated to the rest of the issue, but the code below simply assumes that the input is a `Name` which isn't great...
       return;
     }
     switch (style.name) {

[timvandermeij]

Thank you for the triage. It looks acceptable to me, provided that we add a comment above the if (isName(width)) { line saying something along the lines of "Some corrupt documents provide the width as a Name instead of a number (fixes #10385)." and we add a unit or reference test.

[THausherr]

This change is not really correct - if manually changing /W to /5, we don't get a thick border with Adobe, we still have an invisible border. But IMHO Adobe is wrong: "The border width in points. If this value is 0, no border shall drawn. Default value: 1." A name is like "nothing", so default 1 should be used. CVMG News-5201-Jan 2019-p8.pdf

[timvandermeij]

So if it's a Name object, we should always draw nothing instead of the current change where we try to parse the Name's value?

[THausherr]

Yes. (or use 0, which mean "draw nothing"). That would replicate the current Adobe behavior.

[timvandermeij]

Fixed now. Thank you for the feedback, @THausherr!