mozilla / pdf.js

PDF Reader in JavaScript
https://mozilla.github.io/pdf.js/
Apache License 2.0
47.85k stars 9.91k forks source link

missing support for "view" parameter in URL fragment identifiers #10773

Open markmatney opened 5 years ago

markmatney commented 5 years ago

According to Parameters for Opening PDF Files (version 9.0) (and referenced in RFC 8118):

view=Fit view=FitH view=FitH,top view=FitV view=FitV,left view=FitB view=FitBH view=FitBH,top view=FitBV view=FitBV,left

Set the view of the displayed page, using the keyword values defined in the PDF language specification. For more information, see the PDF Reference. Scroll values left and top are floats or integers in a coordinate system where 0,0 represents the top left corner of the visible page, regardless of document rotation.

It appears that the view parameter in PDF URL fragment identifiers is currently unsupported. To verify, I tested as follows:


Configuration:

Steps to reproduce the problem:

  1. Check out the repository, run npm install and gulp server, then navigate the viewer to a PDF that isn't automatically zoomed in to fill your viewport width on viewer initialization (http://localhost:8888/web/viewer.html?file=%2Ftest%2Fpdfs%2Ftracemonkey.pdf works for me).
  2. Append #page=1&view=FitH to the URL in the address bar, and reload the page.

What is the expected behavior?

The viewer should set the zoom level so that the PDF fills the entire width of the viewport.

What went wrong?

The viewport was not transformed as expected.


It looks like the problem is mainly with PDFLinkService.setHash. There is no check for the view parameter.

Interestingly enough, Fit* values are being checked for in the zoomparameter, but I'm having trouble understanding why (since none of RFC 3778, RFC 8118, or the PDF Open Parameters document say that those are vallid values for zoom). The deepest I was able to dig was this PR, but it looks like Fit* values have been handled this way even before that.

residentevil1256 commented 4 years ago

Hello, I am working on a project for a software course and I would love to work on this. I noticed that the Fit* values work for the zoom parameter. I want to make sure that adding support for these values for the view parameter is still a desired feature. If so, I would appreciate any additional information anyone has on this since it is my first time contributing to a public Open-Source project. Thank you.

timvandermeij commented 4 years ago

If I look at the specification at https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/pdf_open_parameters_v9.pdf#page=6 I also think that this wasn't implemented correctly originally. The most important point is that we should always follow the specification. I don't know much about this PDF feature itself, but it indeed looks like the Fit* functionality that was added to the zoom parameter should indeed have been the view parameter. Feel free to work on this to make this adhere to the specification.

markmatney commented 4 years ago

Hi @residentevil1256, I just wanted to let you know that this is still a desired feature, and that I think you should be aware of https://github.com/mozilla/pdf.js/issues/2843 which documents a confusing bug in the current implementation of this feature (called "zoom" by PDFjs, but actually "view" according to "Parameters for Opening PDF Files").

huang325 commented 4 years ago

Hi @markmatney , I'm currently looking into this feature as part of course project. I took a look into what you mentioned in #2843 comment , such that there should be a transformation from the "open parameters" coordinate system to the "user space" coordinate system.

However, based on my understanding, BaseViewer.scrollPageIntoView is actually seeking for "open parameters" and will convert it to "user space" at here.

Thus, the current issue is that BaseViewer.scrollPageIntoView desires a from-botton-left value but we are giving it a from-top-left value.

My current plan is make changes in BaseViewer.js. Such as changing this line to be y = pageHeight - destArray[2]; then change this and this lines to be y = pageHeight - destArray[3];.

I tested on some cases and this solution works. But just want to double check if this is the proper way of solving it. Here are my attempted changes.

ousia commented 1 year ago

@timvandermeij, as you suggested, I think the values from view and zoom may be split such as in:

if (params.has("view")) {
  const viewArgs = params.get("view").split(",");
  const viewArg = viewArgs[0];

  if (viewArg === "Fit" || viewArg === "FitB") {
    dest = [null, { name: viewArg }];
  } else if (
    viewArg === "FitH" ||
    viewArg === "FitBH" ||
    viewArg === "FitV" ||
    viewArg === "FitBV"
  ) {
    dest = [
      null,
      { name: viewArg },
      viewArgs.length > 1 ? viewArgs[1] | 0 : null,
    ];
  } else if (viewArg === "FitR") {
    if (viewArgs.length !== 5) {
      console.error(
        'PDFLinkService.setHash: Not enough parameters for "FitR".'
      );
    } else {
      dest = [
        null,
        { name: viewArg },
        viewArgs[1] | 0,
        viewArgs[2] | 0,
        viewArgs[3] | 0,
        viewArgs[4] | 0,
      ];
    }
  } else {
    console.error(
      `PDFLinkService.setHash: "${viewArg}" is not a valid view value.`
    );
  }
}

if (params.has("zoom")) {
  // Build the destination array.
  const zoomArgs = params.get("zoom").split(","); // scale,left,top
  const zoomArg = zoomArgs[0];
  const zoomArgNumber = parseFloat(zoomArg);

  // If the zoomArg is a number, it has to get divided by 100.
  dest = [
    null,
    { name: "XYZ" },
    zoomArgs.length > 1 ? zoomArgs[1] | 0 : null,
    zoomArgs.length > 2 ? zoomArgs[2] | 0 : null,
    zoomArgNumber ? zoomArgNumber / 100 : zoomArg,
  ];
}

If this is right (I have basically copied and pasted text), I don't mind to submit a PR. But consider that I cannot code properly (very basic Lua exposure and no proper JavaScript).

Many thanks for your help.