seanaye/chore/pull upstream #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nType-STREAM Remove the unused `CMapCompressionType.STREAM` value
…ToolbarButton Simplify the `dropdownToolbarButton`-select width computation
…uest Clean-up the data that we're sending with "GetDocRequest"
…correctly (issue 15557) *Fixes a regression from PR 15246, sorry about that!* The return value of all `Annotation.getOperatorList` methods was changed in PR 15246, however I missed updating the error code-path in `Page.getOperatorList` which thus breaks all operatorList-parsing for pages with corrupt Annotations.
Ensure that `Page.getOperatorList` handles Annotation parsing errors correctly (issue 15557, bug 1794351)
Note how after having found the "%PDF-" prefix we then read both the prefix and the version in the loop, only to then remove the prefix at the end. It seems better to instead advance the stream position past the "%PDF-" prefix, and then read only the version data. Finally the loop-condition can also be simplified slightly, to further clean-up some very old code.
Slightly re-factor the version fetching in `PDFDocument.checkHeader`
…ch `AnnotationEditorUIManager`-instance (issue 15564) When a new PDF document is opened in the GENERIC viewer we (obviously) create a new `AnnotationEditorUIManager`-instance, since those are document-specific, and thus we need to ensure that we actually register the `editorTypes` for each one.
[GENERIC viewer] Ensure that the we register the `editorTypes` for each `AnnotationEditorUIManager`-instance (issue 15564)
…editor isn't empty (bug 1794717) When a text editor is empty, clicking outside will create a new editor, hence it makes sense to keep a caret cursor.
[Editor] Change the caret cursor into the arrow one only when a text editor isn't empty (bug 1794717)
… CFF fonts (issue 15559) *Please note:* I don't really know what I'm doing here, however the patch appears to fix the referenced issue when comparing the rendering with Adobe Reader (with the caveat that I don't speak the language in question).
…GetOperatorList-handling This code was added all the way back in PR 6698, almost seven years ago, for backwards compatibility reasons. At this point in time, it seems that we can remove that since: - We have more fine-grained "UnsupportedFeature" reporting elsewhere in the worker-thread code nowadays. - The GetOperatorList-handling is now using `ReadableStream`s, which means that errors are being forwarded to the main-thread anyway. - We're also no longer displaying a notification-bar, in the *built-in* Firefox PDF Viewer, for any of these "UnsupportedFeature" messages.
- Fix Field::getArray in order to collect only the fields which have a value;
- Fix AFSimple_Calculate:
* allow to have a string with a list of field names as argument;
* since a field can be non-terminal, use Field::getArray to collect
the field under it and then apply the calculation on all the descendants.
Given the sheer number of heuristics added to this method over the years, moving the *valid* unicode found case to the top should improve readability of the code.
[JS] Take into account all the required fields for some computations
…atorList-UnsupportedFeature [api-minor] Stop sending "UnsupportedFeature" from the worker-thread GetOperatorList-handling
Take the /CIDToGIDMap into account when getting the glyph mapping for CFF fonts (issue 15559)
…s used with pattern fill (bug 1795263, mozilla#15573)
Use all the current transform as key when caching some image for masks used with pattern fill (bug 1795263, mozilla#15573)
Note how we're currently skipping all main-thread cleanup when document destruction has started, but for some reason we're still dispatching the "Cleanup" message. This seems like a simple oversight, since destruction will already invoke the `BasePdfManager.cleanup` method (on the worker-thread) to fully clear-out all caches.
…royed Don't trigger worker-thread cleanup when destruction has already started
Part of this is very old code, and back when support for parsing the catalog-version was added things became less clear (in my opinion). Hence this patch tries to improve things, by e.g. validating the header- and catalog-version separately.
…de-refactor Slightly re-factor `PartialEvaluator._simpleFontToUnicode`
Re-factor the PDF version parsing in the worker-thread
[Editor] Ink editors must have their dimensions in percents after having been resized
Given that the new sidebar icon is slightly shorter than the old one, it cannot hurt to ever so slightly tweak the vertical position of the notification icon. (While the patch also changes the CSS rule used for the horizontal position, this is a no-op and was done to improve consistency between the two values.)
…ender-async Change the `XfaLayerBuilder.render` method to be asynchronous
Set the dimensions of the various layers at their creation
…ny annotations, on zooming and rotation For pages without any annotations, applies e.g. to the `tracemonkey.pdf` document, we'll repeatedly try to re-create the `annotationLayer` on every zoom and rotation operation. The reason that this happens is because we don't insert the `annotationLayer`-div into the DOM unless there's annotations present on the page, which thus means that we miss the existing `annotationLayer`-caching present in the `PDFPageView` implementation. This is a very old issue, and the easiest solution is to simply always insert an *empty* (and hidden) `annotationLayer`-div such that the existing code/caching starts working for the "no annotations" case as well. Note that this is consistent with other layers, since e.g. the `textLayer` and/or `annotationEditorLayer` may be empty. Given that only a limited, by default ten, number of pages are ever active at once the additional DOM-elements shouldn't effect things negatively here.
This was essentially done only to compensate for the viewer calling `PDFPageProxy.getAnnotations` unconditionally on every annotationLayer-rendering invocation. With the previous patch that's no longer happening, and this API-caching should thus no longer be necessary.
…ilder-no-annotations Don't attempt to re-create the `annotationLayer`, for pages without any annotations, on zooming and rotation
…n-scaled, viewport dimensions While reviewing recent patches, I couldn't help but noticing that we now have a lot of call-sites that manually access the `PageViewport.viewBox`-property. Rather than repeating that verbatim all over the code-base, this patch adds a lazily computed and cached getter for this data instead.
[api-minor] Add a new `PageViewport`-getter to access the original, un-scaled, viewport dimensions
…ts (follow-up of mozilla#15770) In order to move the annotations in the DOM to have something which corresponds to the visual order, we need to have their dimensions/positions which means that the parent must have some dimensions.
The annotation layer dimensions must be set before adding some elements (follow-up of mozilla#15770)
|
@seanaye Based on what I see amongst the massive amount of changes, they essentially did a rewrite on some parts and moved a lot of files into other portions so changes that we've made to will no longer exist in the proper place (as they are considered new files), including the glyph positions. This will also likely affect some types used in our app, since the JSDoc signatures are used to generate the typescript used. The net result of the changes we've made can be seen here: We need to ensure that these changes are ported over correctly. After they are ported, functionality reliant on our changes still function correctly, namely:
Once this functionality has been QA'd and general PDF behavior functions correctly, I think it's relatively safe to move forward with the PDF.js-based decryption. |
|
It's also worth noting that some functionality that we use to remove flickering when selecting text is permanently removed upstream: It's worth checking if selecting text across multiple paragraphs with sizable gaps between them flickers in this updated version, because we might have to figure out a better fix than simply re-integrating the That said, I feel like probably just adding enhanceTextSelection back into the code is the right fix, but introduces a greater maintenance burden long term as things diverge. It seems that it certainly makes Chromium-based selection less pleasant, and the alternative textLayerMode mentioned also no longer exists: Demonstration of said issue in latest version: mozilla#15733 |
|
Also of note, there are likely changes that were done to the find controller. We extend and override the find controller class here: The methods that have been overridden to support our functionality may have to be changed. |
|
@chase yea, not done yet I've been working on release stuff. The commit has been kinda "dropped" since the changes were reverted in later commits anyway. I'll do it properly tmr tho |
seanaye
force-pushed
the
seanaye/chore/pull-upstream
branch
from
8730c1e to
5d4569a
Compare
seanaye
force-pushed
the
seanaye/chore/pull-upstream
branch
from
5d4569a to
877cdd0
Compare
|
Closed in favor of #3 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull in upstream changes.
I think I did properly