Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406) #18417

Snuffleupagus · 2024-07-10T08:15:26Z

This functionality is purposely limited to development mode and GENERIC builds, since it's unnecessary in e.g. the built-in Firefox PDF Viewer, and will only be used when a <base>-element is actually present.

Please note: We also have tests in mozilla-central that will indirectly ensure that relative filter-URLs work as intended in the Firefox PDF Viewer, see https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test/browser_pdfjs_filters.js

To test that the issue is fixed, the following code can be used:

<!DOCTYPE html>
<html>
<head>
  <meta charset="UTF-8">
  <base href=".">
  <title>base href (issue 18406)</title>
</head>
<body>

<ul>
  <li>Place this code in a file, named `base_href.html`, in the root of the PDF.js repository</li>
  <li>Run <pre>npx gulp dist-install</pre></li>
  <li>Run <pre>npx gulp server</pre></li>
  <li>Open <a href="http://localhost:8888/base_href.html">http://localhost:8888/base_href.html</a> in a browser</li>
  <li>Compare rendering with <a href="http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf">http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf</a></li>
</ul>

<canvas id="the-canvas" style="border: 1px solid black; direction: ltr;"></canvas>

<script src="/node_modules/pdfjs-dist/build/pdf.mjs" type="module"></script>

<script id="script" type="module">
  //
  // If absolute URL from the remote server is provided, configure the CORS
  // header on that server.
  //
  const url = '/test/pdfs/issue16287.pdf';

  //
  // The workerSrc property shall be specified.
  //
  pdfjsLib.GlobalWorkerOptions.workerSrc =
    '/node_modules/pdfjs-dist/build/pdf.worker.mjs';

  //
  // Asynchronous download PDF
  //
  const loadingTask = pdfjsLib.getDocument(url);
  const pdf = await loadingTask.promise;
  //
  // Fetch the first page
  //
  const page = await pdf.getPage(1);
  const scale = 1.5;
  const viewport = page.getViewport({ scale });
  // Support HiDPI-screens.
  const outputScale = window.devicePixelRatio || 1;

  //
  // Prepare canvas using PDF page dimensions
  //
  const canvas = document.getElementById("the-canvas");
  const context = canvas.getContext("2d");

  canvas.width = Math.floor(viewport.width * outputScale);
  canvas.height = Math.floor(viewport.height * outputScale);
  canvas.style.width = Math.floor(viewport.width) + "px";
  canvas.style.height = Math.floor(viewport.height) + "px";

  const transform = outputScale !== 1
    ? [outputScale, 0, 0, outputScale, 0, 0]
    : null;

  //
  // Render PDF page into canvas context
  //
  const renderContext = {
    canvasContext: context,
    transform,
    viewport,
  };
  page.render(renderContext);
</script>

</body>
</html>

Snuffleupagus · 2024-07-10T08:30:43Z

@calixteman How do you feel about this, do you think this is something that we should support?

calixteman · 2024-07-10T08:52:03Z

From an api pov, I think it isn't that bad to try to be environment agnostic: it makes the pdfjs integration easier especially in such a case it's very uneasy to make the link between a rendering issue and the presence of <base>.
That said, I'd prefer having a test here or at least we should have a very basic html file (attached to the original bug) to reproduce the issue in order to have something to manually test if someone wants to touch this part of the code.
I think I already wrote something about that in the past, but I think we should have a kind of mix of integration and reference test: we could load a html in an integration test, get an image either from a canvas or from a screenshot and compare it with a reference.

Snuffleupagus · 2024-07-10T10:06:21Z

That said, I'd prefer having a test here or at least we should have a very basic html file (attached to the original bug) to reproduce the issue in order to have something to manually test if someone wants to touch this part of the code.

I've created a basic standalone test-case, please see #18417 (comment) which is also included in the commit message.

Snuffleupagus · 2024-07-10T10:16:53Z

/botio-windows test

Snuffleupagus · 2024-07-10T10:45:27Z

/botio-linux browsertest

…18406) This functionality is purposely limited to development mode and GENERIC builds, since it's unnecessary in e.g. the *built-in* Firefox PDF Viewer, and will only be used when a `<base>`-element is actually present. *Please note:* We also have tests in mozilla-central that will *indirectly* ensure that relative filter-URLs work as intended in the Firefox PDF Viewer, see https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/test/browser_pdfjs_filters.js --- To test that the issue is fixed, the following code can be used: ```html <!DOCTYPE html> <html> <head> <meta charset="UTF-8"> <base href="."> <title>base href (issue 18406)</title> </head> <body> <ul> <li>Place this code in a file, named `base_href.html`, in the root of the PDF.js repository</li> <li>Run <pre>npx gulp dist-install</pre></li> <li>Run <pre>npx gulp server</pre></li> <li>Open <a href="http://localhost:8888/base_href.html">http://localhost:8888/base_href.html</a> in a browser</li> <li>Compare rendering with <a href="http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf">http://localhost:8888/web/viewer.html?file=/test/pdfs/issue16287.pdf</a></li> </ul> <canvas id="the-canvas" style="border: 1px solid black; direction: ltr;"></canvas> <script src="/node_modules/pdfjs-dist/build/pdf.mjs" type="module"></script> <script id="script" type="module"> // // If absolute URL from the remote server is provided, configure the CORS // header on that server. // const url = '/test/pdfs/issue16287.pdf'; // // The workerSrc property shall be specified. // pdfjsLib.GlobalWorkerOptions.workerSrc = '/node_modules/pdfjs-dist/build/pdf.worker.mjs'; // // Asynchronous download PDF // const loadingTask = pdfjsLib.getDocument(url); const pdf = await loadingTask.promise; // // Fetch the first page // const page = await pdf.getPage(1); const scale = 1.5; const viewport = page.getViewport({ scale }); // Support HiDPI-screens. const outputScale = window.devicePixelRatio || 1; // // Prepare canvas using PDF page dimensions // const canvas = document.getElementById("the-canvas"); const context = canvas.getContext("2d"); canvas.width = Math.floor(viewport.width * outputScale); canvas.height = Math.floor(viewport.height * outputScale); canvas.style.width = Math.floor(viewport.width) + "px"; canvas.style.height = Math.floor(viewport.height) + "px"; const transform = outputScale !== 1 ? [outputScale, 0, 0, outputScale, 0, 0] : null; // // Render PDF page into canvas context // const renderContext = { canvasContext: context, transform, viewport, }; page.render(renderContext); </script> </body> </html> ```

moz-tools-bot · 2024-07-11T09:32:23Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/77ac47c5ba4c015/output.txt

moz-tools-bot · 2024-07-11T09:32:23Z

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/c5f2dbf1d73429e/output.txt

calixteman

LGTM. Thank you.

moz-tools-bot · 2024-07-11T10:02:03Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/77ac47c5ba4c015/output.txt

Total script time: 29.67 mins

Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 13
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/77ac47c5ba4c015/reftest-analyzer.html#web=eq.log

moz-tools-bot · 2024-07-11T10:16:03Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/c5f2dbf1d73429e/output.txt

Total script time: 43.66 mins

Unit tests: FAILED
Integration Tests: FAILED
Regression tests: FAILED

  different ref/snapshot: 4

Image differences available at: http://54.193.163.58:8877/c5f2dbf1d73429e/reftest-analyzer.html#web=eq.log

…baseUrl Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406)

Snuffleupagus added the core label Jul 10, 2024

Snuffleupagus requested a review from calixteman July 10, 2024 08:15

Snuffleupagus force-pushed the DOMFilterFactory-baseUrl branch from 4905386 to 7aac2bf Compare July 10, 2024 09:52

Snuffleupagus linked an issue Jul 10, 2024 that may be closed by this pull request

[Bug]: Adding a <base href> tag to a page containing a pdfjs viewer breaks transparancy filters in Firefox. #18406

Closed

Snuffleupagus force-pushed the DOMFilterFactory-baseUrl branch 2 times, most recently from 6b818f8 to 8a7884c Compare July 10, 2024 10:12

Snuffleupagus marked this pull request as ready for review July 10, 2024 10:15

Snuffleupagus force-pushed the DOMFilterFactory-baseUrl branch from 8a7884c to 50a5a15 Compare July 11, 2024 09:30

mozilla deleted a comment from moz-tools-bot Jul 11, 2024

calixteman approved these changes Jul 11, 2024

View reviewed changes

Snuffleupagus merged commit 8082c2d into mozilla:master Jul 11, 2024
9 checks passed

Snuffleupagus deleted the DOMFilterFactory-baseUrl branch July 11, 2024 10:25

stephanrauh pushed a commit to stephanrauh/pdf.js that referenced this pull request Jul 14, 2024

Merge pull request mozilla#18417 from Snuffleupagus/DOMFilterFactory-…

2938bbd

…baseUrl Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406)

Snuffleupagus mentioned this pull request Jul 16, 2024

[api-minor] Remove Outliner from the official API #18441

Merged

npanchal108 mentioned this pull request Aug 31, 2024

[Snyk] Upgrade pdfjs-dist from 4.0.269 to 4.5.136 npanchal108/ecommercebase#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406) #18417

Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406) #18417

Snuffleupagus commented Jul 10, 2024 •

edited

Loading

Snuffleupagus commented Jul 10, 2024

calixteman commented Jul 10, 2024

Snuffleupagus commented Jul 10, 2024 •

edited

Loading

Snuffleupagus commented Jul 10, 2024

Snuffleupagus commented Jul 10, 2024

moz-tools-bot commented Jul 11, 2024

moz-tools-bot commented Jul 11, 2024

calixteman left a comment

moz-tools-bot commented Jul 11, 2024

moz-tools-bot commented Jul 11, 2024

Create absolute filter-URLs when needed in DOMFilterFactory (issue 18406) #18417

Create absolute filter-URLs when needed in DOMFilterFactory (issue 18406) #18417

Conversation

Snuffleupagus commented Jul 10, 2024 • edited Loading

Snuffleupagus commented Jul 10, 2024

calixteman commented Jul 10, 2024

Snuffleupagus commented Jul 10, 2024 • edited Loading

Snuffleupagus commented Jul 10, 2024

Snuffleupagus commented Jul 10, 2024

moz-tools-bot commented Jul 11, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented Jul 11, 2024

From: Bot.io (Windows)

Received

calixteman left a comment

Choose a reason for hiding this comment

moz-tools-bot commented Jul 11, 2024

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented Jul 11, 2024

From: Bot.io (Windows)

Failed

Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406) #18417

Create absolute filter-URLs when needed in `DOMFilterFactory` (issue 18406) #18417

Snuffleupagus commented Jul 10, 2024 •

edited

Loading

Snuffleupagus commented Jul 10, 2024 •

edited

Loading