Functionality wise, `ensureThumbnailVisible` is essentially just a shorthand for `scrollThumbnailIntoView`. (And note that `PDFViewer` doesn't implement a `ensurePageVisible` method.)
The only remaining usage of `PDFThumbnailViewer_ensureThumbnailVisible` is inside `PDFPresentationMode`, which introduces an otherwise unnecessary `PDFThumbnailViewer` dependency there.
We're already relying on the `presentationmodechanged` event, in various files, to track the state of Presentation Mode. Thus we can simply listen for that event in `PDFSidebar` too, and update the thumbnails if necessary.
The sidebar code has, except for minor fixes/additions (such as attachments), been largely untouch for years.
To avoid having a bunch of sidebar code sprinkled throughout viewer.js, this patch moves the sidebar code into a separate file (pdf_sidebar.js), similar to how most other functionality has been moved in the last few years.
Besides simply moving code around, this patch also has the added benefit that we now keep track of the sidebar state (not just opened/closed).
This now makes it possible to handle both `Preferences` *and* `ViewHistory` settings for the sidebar state in a cleaner way, preventing strange and confusing interactions between the two.
Changes `PDFOutlineView`/`PDFAttachmentView` to be initialized once, since we're always creating them, and refactor their `render` methods to instead pass in the `outline`/`attachments`.
For consistency with other "classes", the `PDFOutlineView`/`PDFAttachmentView` are renamed to `PDFOutlineViewer`/`PDFAttachmentViewer`.
Also, make sure that the outline/attachments are reset when the document is closed. Currently we keep the old ones around until the `getOutline`/`getAttachments` API calls are resolved for a new document.
To reduce code duplication, the initialization code now uses the `reset` method.
Also, this patch moves `charactersToNormalize` out of `PDFFindController`, since it seemed better suited to be a "constant".
This is required to be able to use it in the annotation display code,
where we now apply it to sanitize the filename of the FileAttachment
annotation. The PDF file from https://bugzilla.mozilla.org/show_bug.cgi?id=1230933 has shown that some PDF generators include the path of the file rather than the filename, which causes filenames with weird initial characters. PDF viewers handle this differently (for example Foxit Reader just replaces forward slashes with spaces), but we think it's better to only show the filename as intended.
Additionally we add unit tests for the `getFilenameFromUrl` helper
function.
Currently this constant is present in `viewer.js`, but it is not used there at all. Instead, it is used in `view_history.js` where we have a global for it. We might as well move the constant to `view_history.js` as that is the only place where it is used, thereby removing a global and an unused constant from `viewer.js`.
This issue is actually, in a sense, "caused" by the fact that the API/viewer supports partial loading/rendering. Previously when the *entire* document was always fetched before rendering begun, we knew all page sizes in advance and this issue didn't exist.
Now we use the size of *one* page in order to set the initial size of every page, until we've fetched the pages and thus know their correct sizes.
This means that during loading the size of the pages can change, which may cause the initial position to become scrolled out of view.
The most naive solution to this problem would perhaps be to delay setting the initial position on load for all documents, until all pages are fetched. However I think that would be a *really* bad idea, since doing so would make the initial rendering slower and make it feel sluggish for most documents.
Since there is generally no way of knowing if a document has different sized pages prior to loading it, we can only check once the pages are available.
Hence this patch, which treats documents with different sized pages as a special case, by re-applying the initial position when all pages have become available.
- This commit adds a '>' before every outline item that has subitems.
- Click on the '>' to collapse all subitems under that item (which turns
the '>' in a 'v').
- Shift + click expands/collapses all descendant items under that tree.
- Double-clicking on the "Show Document Outline" button in the toolbar
expands/collapses all outline items.
Currently in `switchSidebarView` there is code that attempts to handle the `outline` or `attachments` being disabled. However, given the placement of it, that code does not actually accomplish anything. Even more strange is the fact that the way that the buttons are disabled, that code won't ever be hit.
(Looking back with `git blame`, it seems that it has never worked as you'd expect.)
Hence this patch, which correctly disables switching to the `outline` or `attachments` views when they are disabled.
Once PR 6314 restores the 'pagemode' hash parameter, this patch thus makes it impossible to switch to a disabled sidebar view by modifying the 'pagemode' hash of an already loaded document.
This regressed in 0ef6212b64.
Since the 'pagemode' hash parameter requires certain viewer functionality (e.g. thumbnails and an outline) in order to work, it seemed reasonable to move the functionality from `pdf_link_service.js` into `viewer.js`.
Similar to `namedaction`, this patch makes use of an event to forward the 'pagemode' parameter.
Since the zoom value should be in percent, using `PDFViewer.currentScale` will be wrong by a factor of 100, potentially causing the zoom level of the document to become wrong on load.
*This fixes a regression from PR 6192.*
Under some circumstances, the `resize` event handler in `viewer.js` is fired before the scale has been set. This can lead to PDF documents being rendered at the wrong zoom level when they are opened.
It seems that a way to reliably trigger this is to, using the Firefox addon, open a PDF file that triggers the `fallback` bar, in a new background tab (i.e. middle clicking on a link, or use the context menu).
Prior to PR 6192, we checked the selected option in the `scaleSelect` dropdown instead. Since `pageAutoOption` is selected by default in `viewer.html`, this should explain why the issue wasn't visible previously.
Fixes#6106
To avoid future regressions, two new unit tests were added:
1. A new PDF based on the report from #6106, which contains an
OpenAction of type JavaScript and a string "this.print({...}".
2. An existing PDF from https://bugzil.la/1001080 (from #4698).
Although it does not matter, since we don't execute the JavaScript code,
I have also changed "print(true)" to "print({})" since the print method
takes an object (not a boolean). See "Printing PDF documents", page 62:
http://adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/js_developer_guide.pdf
The special handling of the 'custom' scale value is only relevant for the `scaleSelect` dropdown in the standard viewer, hence I think that it should be placed in `viewer.js` instead.
After the creation of `PDFViewer`, its `_resetView` method takes care of resetting, among other things, the page number property. Hence we don't need to set `pdfViewer.currentPageNumber = 1;` here any more, and the comment is no longer accurate either.
*With this patch we're getting very close to fixing 6158.*
The only use-case for `PDFViewerApplication.updateScaleControls` is to try and avoid calling `selectScaleOption` from the `scalechange` event handler in viewer.js.
This will *only* happen when the user has manually changed the scale by using the `<select>` dropdown, which means that in reality this is just a micro optimization. Furthermore, `selectScaleOption` is only skipped for the "named" scale values (e.g. `auto`, `page-actual`, `page-fit`, `page-width`), thus further reducing the value of this code.
Also, since we're updating the scale `<select>` dropdown from an event handler, we're currently depending on the event being dispatched (and handled) completely before the next `scalechange` event. Relying on the execution order of the code in this way, even though it currently works, seems unfortunate since it *could* potentially cause the internal scale value and the UI from getting out of sync.
*This is the next step towards fixing 6158.*
This patch removes the dependency on the state of the scale `<select>` dropdown from the `resize` event handler, and instead uses the (in `PDFViewer`) stored `currentScaleValue`.
I believe that the way this code is currently written is purely for historical reasons, since originally *only* the numerical scale was stored internally (hence there was no other way to access the scale value).
However, since we now store the scale value, we should use it instead of quering the DOM. This helps ensure that the internally stored scale value is always accurately displayed in the UI (which should be good since, after the creation of `PDFViewer`, the `<select>` DOM element is now updated by an event handler).
*The next step towards fixing issue 6158.*
We can just as well access `pdfViewer.currentScaleValue` directly in `PDFViewerApplication`, instead of having a helper function which just acts as a wrapper for it.
This patch is the the first step towards to addressing issue 6158, which will be done by refactoring the code for setting/getting the current scale in `viewer.js`.
Before this patch, zooming in/out via the scroll wheel caused the page
to be zoomed relative to the upper-left corner of the page, i.e. the
upper-left corner of the page stays at a fixed position.
After this patch, the page is zoomed relative to the cursor position,
i.e. after zooming in/out, the part under the cursor 'has not moved'.
This only applies when the page does not fit in the viewport, because
pages smaller than the viewpoer are always centered.
Yury Delendik
Tobias Schneider
Jonas Jenwald
Tim van der Meij
Sebastian Hengst
Manas
Andy Parisi
Rob Wu