Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] master from mozilla:master #96

Merged
merged 152 commits into from
Sep 24, 2023
Merged

[pull] master from mozilla:master #96

merged 152 commits into from
Sep 24, 2023

Conversation

pull[bot]
Copy link

@pull pull bot commented Sep 4, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

dependabot bot and others added 3 commits September 4, 2023 12:51
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
When the annotation has a popup then the popup can be toggled in using
the Enter key and hidden in using the Escape key.
Make annotations focusable (bug 1851489)
@pull pull bot added the ⤵️ pull label Sep 4, 2023
calixteman and others added 26 commits September 5, 2023 10:41
…bug 1851498)

The goal is to always have something which is focusable to let the user select
it with the keyboard.
It fixes the mentioned bug because, the annotation layer will now have a container
to attach the canvas for annotations having their own canvas.
Unconditionally render non-form annotations in the annotation layer (bug 1851498)
While reviewing PR 16898 it occurred to me that it's currently impossible to trigger downloading of FileAttachment annotations using the keyboard.
Hence this patch adds `Ctrl + Enter` as the keyboard shortcut to download those, thus supplementing the existing double-clicking when using a mouse.
…download

Support downloading FileAttachment annotations with the keyboard
Focus callback must be called only when the element has been blurred.
For example, blur callback (which implies some potential validation) is not called
because the newly focused element is an other tab, an alert dialog, ... so consequently
the focus callback mustn't be called when the element gets its focus back.
Only call the focus/blur callbacks when it's necessary (bug 1851517)
…ons/checkout-4

Bump actions/checkout from 3 to 4
[Editor] Avoid to use parent of editors in destroyed pages
The classes were stripped out during when creating the field name but
it led to a wrong name.
Since class components in a path are irrelevant, they're just ignored
when searching for a node in the datasets.
Construct the correct field name and strip out classes when searching
…g 1845087)

The tag id will be useful in order to update the StructTree when saving
the pdf.
Over time the amount of "document level" data potentially needed during parsing of Annotations have increased a fair bit, which means that we currently need to ensure that a bunch of data is available for each individual Annotation.
Given that this data is "constant" for a PDF document we can instead create (and cache) it lazily, only when needed, *before* starting to parse the Annotations on a page. This way the parsing of individual Annotations should become slightly less asynchronous, which really cannot hurt.

An additional benefit of these changes is that we can reduce the number of parameters that need to be explicitly passed around in the annotation-code, which helps overall readability in my opinion.

One potential drawback of these changes is that the `AnnotationFactory.create` method no longer handles "everything" on its own, however given how few call-sites there are I don't think that's too much of a problem.
…nary`

While it makes sense to check that the `destDict` parameter is indeed a Dictionary, since that data comes from the PDF document itself, the `resultObj` parameter is an internal PDF.js implementation detail that should always be correct (or tests will fail).
[Editor] Add the parent tag id (if any) to the serialized editors (bug 1845087)
It'll avoid to have the duplication of the code to get the encrypt transform,
and last but not least, it'll avoid to forget about encryption.
Slightly reduce asynchronicity when parsing Annotations
Update packages and translations
The unit test is re-enabled because it no longer seems to fail after 10
runs on Linux where this used to fail often. Code inspection also shows
that the code is correct and should raise the previous exception
(anymore). Finally, a lot has changed since this test was disabled such
as new Jasmine versions, new Linux bot OS version and new browser
versions.
Enable unit test "creates pdf doc from non-existent URL"
[Editor] Only get back the focus when it has been lost after an editor has been moved in the DOM
…g it

When I started looking at PR 16938 it occurred to me that some of the new structTree-methods are synchronously accessing certain dictionary-data (not used during "normal" structTree-parsing), which may not be generally safe since everything in a dictionary could be a reference (and the relevant data may not have been loaded yet).

Rather than suggesting that we make all those new methods even more asynchronous, to me the overall simplest and safest solution is to ensure that the *entire* PDF document has been loaded *before* we begin saving it. In practice this shouldn't really affect "performance" of saving noticeably, since it's always depended on the entire PDF document being downloaded.

Finally note that with the exception of the PDF document possibly not having been fully downloaded when saving is triggered, all other "global" document properties are pretty much guaranteed to already be available at this point.
Snuffleupagus and others added 29 commits September 22, 2023 17:35
Don't bother trying to unregister the "reporttelemetry" event listener
[Editor] Tweak the save flow in the alt-text dialog
[Editor] Let the Save button always enabled in the alt-text dialog
This patch addresses an edge-case that'll probably never happen, but it nonetheless seems like something that we want to fix.

Note how we're using the `#currentEditor`-field to prevent opening the dialog when it's already active, and it being reset once the dialog has been closed.
By also resetting the `#currentEditor`-field during destruction, instead of waiting until the dialog has actually closed (assuming it's currently open), there's a *tiny* window of time[1] during which we could theoretically allow to (incorrectly) re-open the dialog and thus getting out-of-sync state in the viewer-component.

---
[1] Since the "close" event, on a dialog-element, is dispatched asynchronously by the browser.
…rrentEditor

Don't reset `this.#currentEditor` when destroying the dialog
…(PR 16987 follow-up)

The dialog element handles closing with <kbd>Esc</kbd> automatically, however we're not reporting telemetry in that case.
In order to fix that the easiest solution, as far as I'm concerned, seem to be moving the telemetry reporting into the dialog-close handler since it's always invoked.
[Editor] Report telemetry when closing the altText dialog with `Esc` (PR 16987 follow-up)
…d dialog

but keep it for the text area.
Disable pointerdown on the alt-text button to disable dragging the editor
when the button is clicked (especially when slightly moving the mouse
between the down and the up).
[Editor] Disable context menu on alt-text button and in the associated dialog
Fix integration test "FreeText Editor FreeText (edit existing in double clicking on it) must move an annotation"
Fix integration test "Interaction in issue15053.pdf must check that a button and text field with a border are hidden"
Currently we duplicate this event handler function in multiple places, which seems unnecessary.
Use one `noContextMenu` function in both the src/- and web/-folders
…editor mustn't catch mouse events (bug 1854818)
[Editor] The ::before containter containing the border of a selected editor mustn't catch mouse events (bug 1854818)
This integration test fails often because we wait for scripting to be
ready before we check the printed page, but most of the time the PDF
is already done printing before scripting is reported to be ready.

This happens because the print trigger is on the `Open` event, which is
one of the first events to be dispatched and, most notably, before
scripting is marked as ready; please see
https://github.com/mozilla/pdf.js/blob/master/web/pdf_scripting_manager.js#L176-L191.
Given that the PDF document is only one page, printing it is usually
finished between triggering the `Open` event and scripting reported
to be ready. If this happens the printed page is already destroyed
before we get to our actual test, which will then timeout because it
will never find the printed page in the DOM.

This commit fixes the problem by not awaiting scripting to be ready
because the fact that the printed page appears is already enough to know
that autoprint was triggered (after all, there is no other user
interaction involved here). While we're here we also switch to the
shorter `page.waitForSelector` function.
Fix integration test "Interaction in autoprint.pdf must check if printing is triggered when the document is open"
[Editor] Add an integration test for the new alt-text flow
In the scripting integration tests we use a few different typing
delays, mostly 100 or 200 milliseconds. According to for example
https://www.typingpal.com/en/documentation/school-edition/pedagogical-resources/typing-speed,
a fast typing speed is around 300 characters per minute, which is 5
characters per second and therefore a delay of 200 milliseconds between
each keystroke. Note that this is already above average, so in practice
the delay will be even larger. Therefore the 100 milliseconds variant
is unrealistically fast and therefore not suitable for the integration
tests which aim to simulate the average user behavior.

On top of that, the quick typing speeds are problematic for the tests
that involve validation alert dialogs appearing during typing. In those
tests a handler is registered to close the dialog once it pops up, but
it takes time for Puppeteer to notice the dialog, trigger the handler
and close it. If the typing delay, which is the delay between the key
down and key up events according to the Puppeteer source code at
https://github.com/puppeteer/puppeteer/blob/master/packages/puppeteer-core/src/cdp/Input.ts#L209-L215,
is too short, the key up event will be fired before the dialog is
closed. In that time the text box we're typing in is not focused, so
when the dialog is closed the `page.type()` call on the text box will
never resolve because the key up event never reached the text box.

This commit aims to fix the issues by converting all 100 millisecond
delays to 200 milliseconds. For instance the "must check input for US
zip format" failed pretty consistently locally before and hasn't failed
anymore with a 200 millisecond delay.
…ocales

*For many non-English locales the translated strings will be longer, which is easy to forget about during development/review.*

Note how for some locales (e.g. Swedish) the altText-button end up looking horizontally "cramped", hence it seems reasonable to add a bit of inline padding to improve this.
[Editor] Add padding to the altText-button to account for different locales
Fix integration test "Interaction in issue14307.pdf (1) must check input for US zip format"
Bump the stable version in `pdfjs.config`
@vlakken vlakken merged commit 57d196e into showpad:master Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants