Built-in DOM properties are slower than plain JS properties.
A few lines before, textContent is assigned as follows:
textDiv.textContent = geom.str;
So replacing textDiv.textContent.length with geom.str.length slightly
improves performance without side effects.
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.
Ordinarily, local files cannot be embedded in a non-local website. Until
this commit, the extension allowed websites to embed local PDF files on
non-local (e.g. http(s)) websites. This unintended feature is now
disabled, to align better with Chrome's existing security policies
(=local file:-URLs cannot be loaded in a tab unless expicitly allowed).
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.
Currently if the zoom level is reset multiple times in a row, i.e. by pressing <kbd>Ctrl</kbd>+<kbd>0</kbd>, the pages can be re-rendered each time even though their size shouldn't change. Whether this happens can depend on the size of the viewer, but documents with pages in landscape mode seem to be very susceptible to this. (An example is: https://wiki.mozilla.org/images/5/55/MobileOpportunity.pdf.)
This can also effect documents with pages in portrait mode, when they are displayed in Presentation Mode.
The reason for this unnecessary re-rendering is that due to limited numerical precision, the new scale value may change in *only* the last decimal place.
This is similar to the already existing, separate, CSS file used for the `textLayer`, and it's necessary in order for the `pageviewer` components example to actually show annotations correctly.
Considering that most methods of `SimpleLinkService` are complete stubs, or practically "useless" considering what they return, we can actually simplify it even more.
*Note:* This depends on the previous patch, that did a small amount of refactoring of `PDFViewer_scrollPageIntoView`, since `PDFViewer.linkService.page` is no longer accessed.
----------
Currently the `pageviewer` components example doesn't work correctly (an error is printed in the console), since no `linkService` is present when the `AnnotationsLayerBuilder` is created.
*Note:* Given that this uses the `SimpleLinkService`, clicking on e.g. internal links won't actually do anything. However, given that internal links (and similar features) are pretty much useless when only *one* page is loaded the `pageviewer` example, I don't think that really matters.
Also, using the complete `PDFLinkService` would require a `PDFViewer` instance. That would significantly complicate the example, thus making it both less clear and less self contained.
Currently in `PDFViewer_scrollPageIntoView`, we're accessing a number of properties in an indirect and overly complicated way. In particular, using `this.linkService.page` is a *very* roundabout way to access `this.currentPageNumber`.
The reason for this appears to be entirely historical, since prior to PR 5361 the code was placed in `PDFPageView` (or `PageView` as it was called at the time).
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.
In `viewer.js` we have code that ensures that the document is re-rendered on zooming and rotation. However, for `components` based viewers this might not work correctly, since there's currently no code which handles that.
Note that there is a good chance that this "just works" in many `components` viewers already, thanks to the `watchScroll` function.
The explanation is that zooming or rotation, most of the time, causes the viewer to change its (scrollable) size, thus triggering `PDFViewer_update` through `PDFViewer_scrollUpdate`.
However, in general there's no guarantee that this will actually work (since zooming and rotation doesn't necessarily change the size of the viewer for all documents), and requiring every viewer `components` implementer to provide methods for this doesn't seem like a great idea.
The reason that this code can be moved is that the `if (this.loading && page === 1)` check, in the `pagechange` event handler in viewer.js, is never satisfied since `this.loading` is not defined in that scope.
This *could* be considered a regression from PR 5295, since prior to that `this.loading` was using the `PDFViewerApplication` scope (or `PDFView` as it were).
However, I don't think that we need to fix that since we've been shipping this code in no less than *three* Firefox releases (uplifted in https://bugzilla.mozilla.org/show_bug.cgi?id=1084158), without breaking the world.
An explanation of why the `pagechange` code works, despite `this.loading === undefined`, is that `set currentPageNumber` (in `PDFViewer`) returns early whenever `this.pdfDocument` isn't set. This check is, for all intents and purposes, functionally equivalent to checking `PDFViewerApplication.loading`.
Hence we can move the page switching code into `PDFViewer`, and also remove `PDFViewerApplication.loading` since it's not used any more.
(The `this.loading` property was added in PR 686, which was before the current viewer even existed.)
*Note:* The changes in this patch should also be beneficial to the viewer `components`, since requiring every implementer to provide their own `pagechange` event handler just to get `PDFViewer.currentPageNumber` to actually work seems like an unnecessary complication.
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1170063.
The bug only mentions the <kbd>Meta</kbd> key, but given that a similar situation can occur for <kbd>Ctrl</kbd>, it seemed reasonable to also handle that case in the same patch.
The only possible caveat with the patch is that because of the use of `shadow`, things won't work perfectly if either of the prefs are changed *while* the viewer is active. In this case a reload is required in order for it to work correctly, but given that the issue this patch fixes should be quite rare anyway, that seems OK.
Currently `PDFLinkService` requires access to a `PDFHistory` instance in order for it to work correctly (and to avoid errors). If we want `PDFLinkService` to be more useful in custom viewers, I don't think that we actually want to force it to have a `PDFHistory` instance.
Hence this patch, which contains a very simply approach to make `PDFHistory` optional.
Reuse the existing getOutputScale() machinery to render and/or rescale
thumbnail images to the native display resolution. Fixes blurry thumbnails on
HiDPI displays making it a lot easier to make out the content.
During A/B testing we found a significant preference (increased engagement
time, reduced downloads of original PDF for local viewing) for the version that
doesn't animate loading thumbnails.
Profiling shows that the transition effects hit at a pessimal time when the
compositor is busy rendering the main PDF page view, and rendering thumbnail
previews, causing the main scroll view to stutter. With the transition removed
scrolling is perceptibly smoother in Chrome and Firefox. We also hypothesize
that the transition effects added to a perception of slowness but presumably
the aforementioned stutter is the primary issue here.
Having recently spent some time staring at the PasswordPrompt, while fixing issue 6010, I felt that the current border style does not really give a good visual indication that the input field actually has focus.
The current appearance was first introduced in PR 3527; but I don't know if having a different border style in Overlay dialogs was intentional, or if it just "happened".
However, given the colour palette used in the viewer UI, I think that using the same border style for all .toolbarFields makes sense.
*This patch addresses an issue I stumbled upon while working on rewriting the history implementation.*
Currently the `updateviewarea` UI event doesn't include the `location`, and in the event handler in viewer.js we instead reach into `PDFViewer` to retrieve it. Not only does this seem conceptually wrong, it also makes the `updateviewarea` event less useful in general since it's currently only possible to access the `location` if you have a reference to the `PDFViewer`.
To me, this simply appears to be an oversight in the implementation, hence this patch which adds `location` to the `updateviewarea` event. It also changes `location` to be a "private" property of `PDFViewer`.
*This is a follow-up to PR 5966.*
Now that https://bugzilla.mozilla.org/show_bug.cgi?id=1151931 has landed, we're unfortunately in a situation where Firefox warns about `unreachable code after return statement` multiple times when using the PDF.js code in `node make server` mode.
I'm really sorry about the churn in this code!
However I don't think it's a good idea to let Firefox continue to printing these warnings in the console, since it increases the risk of missing actual errors.
This patch:
- Simplifies the way fields are passed to the document properties overlay
- Simplifies the way fields are filled internally in the document properties overlay
- Avoids passing a document properties reference to the secondary toolbar
- This patch uncomments a large portion of the Firefox specific code in viewer.js, by using a preprocessor "trick", to enable linting. Doing this actually uncovered some issues, e.g. variables defined multiple times.
- This patch also fixes a spelling error, `adobe pdfwritter` -> `abobe pdfwriter`, in the `KNOWN_GENERATORS` list used when reporting telemetry data in Firefox.
*Please note:* there's still some Firefox specific code that is commented out, but it's usually just one (or two) lines of code. Hence the risk that errors creep in should be much lower, compared to entire code-blocks.
This patch:
- Puts public methods at the top of the file
- Puts private methods below the public methods and marks them with an underscore
- Adds JSDoc comments to the class
- Adds setDocumentAndUrl to avoid having to handle that in `viewer.js`
This patch:
- Passes in a reference to the current PDFThumbnailViewer, which is used to ensure that the current thumbnail becomes visible when exiting PresentationMode.
- Changes the way that the event listeners for the contextmenu items are defined, to avoid passing in a reference to the SecondaryToolbar.
- Ensures that |supportsFullscreen| always returns a boolean.
Currently `supportsFullscreen` will, when the browser supports the fullscreen API, return e.g. `function mozRequestFullScreen()` instead of `true`.
- Simplifies the |click| handler code when PresentationMode is active.
This code has been obsolete ever since PR 2919 landed.
- Removes hack used to workaround a bug in WebKit browsers, which caused |mousemove| events to be fired when the cursor changed.
This was fixed close to a year ago, see http://code.google.com/p/chromium/issues/detail?id=103041.
This patch:
- Adds a couple of utility methods to `PDFViewer` to enable checking `presentationModeState` without cumbersome comparisons.
- Disentangles `PresentationMode` from `PDFHistory` and `HandTool`, by adding event listeners for `presentationmodechanged` to both of them.
- Adds a utility function to `PDFViewerApplication` for requesting PresentationMode.
- Prevents initialization of `PresentationMode` if the browser does not support the fullscreen API.
The line that this patch removes has been commented out since 2012 (!), see commit c8a6a9ba00.
Also, since the patch is trivial, I'm fixing the formatting of a comment to stop [GitHub from marking it with a red color](https://github.com/mozilla/pdf.js/blob/master/web/viewer.html#L394) (and IE from complaining).
After the refactoring in PR 5678, the B2G viewer now prints the following in the console:
```
ReferenceError: DownloadManager is not defined
```
This will obviously not matter once the B2G viewer is replaced with the new components-based one. But until that happens, I think it makes sense to just stub the class to suppress the error.
For consistency, this preference should be placed amongst the others. The current position is purely for "historical" reasons, since it was one of the first preferences that was added to the code.
Since the link is no longer valid, we should remove it from the comment.
(Using the Internet Archive, it seems that the contents of that page was basically identical to the PDF specification.)
In Blink-based browsers, there is a mismatch between document size and
paper size. Even if exactly the same values and unit are used, it is
possible that the printed results takes more pages than expected.
To solve the issue, the page size is set via @page size, and the canvas
and ancestor nodes are assigned a width+height of 100% (=relative to the
page). This change resolves bugs such as blank pages and split pages.
This patch also rearrange the order of the references to the DOM elements of the predefined zoom values, in viewer.js, to correspond with the element order in the current viewer UI. (The previous ordering is a leftover from the previous viewer UI.)
Instead of trying to hack around various browser defects, let's just disable PresentationMode in the affected browsers. This patch:
- Disables PresentationMode in IE11+ when the viewer is embedded; fixes 4711.
Set transformation matrix in (polyfilled) mozPrintCallback when a scale
is applied. Removed _scaleX and _scaleY in favor of _transformMatrix to
emphasize that the caller MUST ensure that the state of the matrix is
correct before `addContextCurrentTransform` is called.
From reading IRC scrollback, it became clear that having to manually call `PDFFindController.resolveFirstPage` in order for the find functionality to work isn't particulary good. Hence this PR, which moves that code into `PDFViewer.setDocument` to make life easier for third-party implementations.
See "Case-insensitive language comparisons per RFC 4646"
https://github.com/fabi1cazenave/webL10n/pull/51
Removed the "patch" in compatibility.js and fixed the bug in webL10n,
because the logic belongs to webL10n instead of PDF.js.
The immediate motivation for this patch is that Chrome 40 converts
navigator.language to lower case: https://crbug.com/454331
We can pass it in using the options object. Note that that this also avoids creating a DownloadManager object for each separate link (instead, having only one is enough).
This includes an optimization to zero the height and width of existing thumbnail canvases, when they are removed and recreated during rotation of the document. (Credit goes to nnethercote, who initially found this in PR 4920.)
In old versions of the viewer, anchor links were used to be able to scroll a page into view. Since we for quite some time have had a `scrollIntoView` function which handles this, the anchor links are no longer necessary and I believe that they can be removed. Doing so will reduce the total number of DOM elements in the viewer.
For most documents this patch will probably not make much of a difference performance wise, but for very long (1000+ pages) documents this could help performance especially on lower powered hardware.
Now that [bug 649849](https://bugzilla.mozilla.org/show_bug.cgi?id=649849) has been fixed, adding support for `-moz-appearance: none`, the arrow is now too close to the text in the zoom box. This is currently only an issue in Nightly, but assuming the patch doesn't get backed out, this will soon affect all versions of Firefox.
The only simple solution I could find seems to be removing `*-appearance: none` rules from the CSS. I haven't been able to find any easier solutions that still looks the same with/without bug 649849.
Currently if you manage to e.g. open the console (with <kbd>Ctrl</kbd>+<kbd>Shift</kbd>+<kbd>K</kbd>) before the viewer is initialized, the following will be printed in the console: `TypeError: pdfViewer is null`.
This doesn't cause any actual errors, but nevertheless it seems like something we should avoid.
Followup to PR 5413.
When a search term isn't found, the background color of the findInput is supposed to change (to red). This is currently not working as intended, because the CSS rule is not being applied correctly. (It seems that this broke in PR 2208.)
This patch also changes the background color to match the one used in the native Firefox findbar, since the old color seemed a bit too pink.
Since `preferenceSidebarViewOnLoad` is an enumeration value, it seems better to initialize it with the default one instead of a boolean.
`preferencesPdfBugEnabled` uses a superfluous "s", which is a typo I made when I introduced it.
Even after PR 5359, switching to/from Presentation mode is still sluggish and I'm still seeing `too much recursion` printed in the console.
I've managed to track down the issue to line https://github.com/mozilla/pdf.js/blob/master/web/page_view.js#L371. It appears that for this particular case, we actually *need* to set `noScroll = true` when calling the `_setScale` function. (Note that it only applies to *this* line, and not to Presentation mode in general.)
Given the amount of refactoring done to the viewer, I'm not sure what the best way to fix this would be. However I'm submitting this patch as an easy workaround for now, but we should probably refactor this to avoid calling a "private" method directly.
This is a regression from ddd3c8fc2f, which prevents changing pages in the Stepper.
(Apart from fixing the bug, for the sake of consistency, I also removed one instance of `parseInt`.)
After PR #5282, the text highlighting in the Font Inspector is very hard to see against a white background. That seems to be an oversight from the mentioned PR, hence this patch fixes that.
As a consequence of merging #5221 it is more likely to have multiple
overlapping selection divs inside the text layer. Because each individual
element gets the selection style applied, the 30%opacity stacks, making a
60% bar visible where the overlap happens.
As proposed by @rocallahan, this can be fixed by making the selection
style solid and setting opacity for the overall layer.
I assume also that this should make the work for the renderer easier, but
was unable to bench it.
Currently when an exception is thrown, we try to reject `workerReadyCapability` with multiple arguments in src/core/api.js. This obviously doesn't work, hence this patch changes that to instead reject with the exception object as is.
In src/core/worker.js the exception is currently (unncessarily) wrapped in an object, so this patch also simplifies that to directly send the exception object instead.
