While the event listener is removed during testing, the `requestAnimationFrame` isn't cancelled and that occasionally shows up when the integration-tests are run on the bots.
Debugging #17931 uncovered a race condition in the way we use the
`waitForEvent` function. Currently the following happens:
1. We call `waitForEvent`, which starts execution of the function body
and immediately returns a promise.
2. We do the action that triggers the event.
3. We await the promise, which resolves if the event is triggered or
the timeout is reached.
The problem is in step 1: function body execution has started, but not
necessarily completed. Given that we don't await the promise, we
immediately trigger step 2 and it's not unlikely that the event we
trigger arrives before the event listener is actually registered in the
function body of `waitForEvent` (which is slower because it needs to be
evaluated in the page context and there is some other logic before the
actual `addEventListener` call).
This commit fixes the issue by passing the action to `waitForEvent` as
a callback so `waitForEvent` itself can call it once it's safe to do so.
This should make sure that we always register the event listener before
triggering the event, and because we shouldn't miss events anymore we
can also remove the retry logic for pasting.
The integration tests are currently not consistent in how they do
copy/pasting: some tests use the `kbCopy`/`kbPaste` functions with
waiting for the event inline, some have their own helper function to
combine those actions and some even call `kbCopy`/`kbPaste` without
waiting for the event at all (which can cause intermittent failures).
This commit fixes the issues by providing a set of four helper functions
that all tests use and that abstract e.g. waiting for the event away
from the caller. This makes the invididual tests simpler and consistent,
reduces code duplication and fixes possible intermittent failures
due to not waiting for events to trigger.
Browsers have an accessibility option that allows user to enforce
a minimum font size for all text rendered in the page, regardless
of what the font-size CSS property says. For example, it can be
found in Firefox under `font.minimum-size.x-western`.
When rendering the <span>s in the text layer, this causes the
text layer to not be aligned anymore with the underlying canvas.
While normally accessibility features should not be worked around,
in this case it is *not* improving accessibility:
- the text is transparent, so making it bigger doesn't make it more
readable
- the selection UX for users with that accessibility option enabled
is worse than for other users (it's basically unusable).
While there is tecnically no way to ignore that minimum font size,
this commit does it by multiplying all the `font-size`s in the text
layer by minFontSize, and then scaling all the `<span>`s down by
1/minFontSize.
This code contains the same bug that the previous commit fixed in
`waitForEvent`, namely that we don't clear the timeout if the event
is triggered. By using the now fixed `waitForEvent` function we not
only deduplicate this code but we also fix this issue so that no
incorrect timeout logs show up anymore.
Debugging #17931, by printing all parts of the event lifecycle including
timestamps, uncovered that some events for which a timeout was logged
actually did get triggered correctly in the browser. Going over the code
and discovering https://stackoverflow.com/questions/47107465/puppeteer-how-to-listen-to-object-events#comment117661238_65534026
showed what went wrong: if the event we wait for is triggered then
`Promise.race` resolves, but that doesn't automatically cancel the
timeout. The tests didn't fail on this because `Promise.race` resolved
correctly, but slightly later once the timeout was reached we would see
spurious log lines about timeouts for the already-triggered events.
This commit fixes the issue by canceling the timeout if the event we're
waiting for has triggered.
Currently errors in `afterAll` are logged, but don't fail the tests.
This could cause new errors during test teardown to go by unnoticed.
Moreover, the integration test use a different reporting mechanism which
also handled errors differently (this is extra reason to do #12730).
This patch fixes the issues by consistently handling errors in
`suiteStarted` and `suiteDone` in both reporting mechanisms.
Fixes#18319.
This integration test contains three issues:
- The `page.bringToFront()` call is not awaited, even though it returns
a promise (see https://pptr.dev/api/puppeteer.page.bringtofront). Note
that in other tests we do this correctly already.
- The `page.waitForSelector()` call at the end is unnecessary because
that exact condition is already checked at the end of the
`waitForImage` function we call just before this line; see
https://github.com/mozilla/pdf.js/blob/master/test/integration/stamp_editor_spec.mjs#L74.
- The pages should be closed in reversed order; please refer to the
description in #18318 for more details.
Fixes#18318.
This integration test is currently the only one that spawns a separate
browser instance. However, while it closes the browser once it's done,
it doesn't close the page (and therefore doesn't call the `testingClose`
method) like the other integration tests do.
This commit fixes this difference by closing the page before closing the
browser, thereby ensuring all regular cleanup logic gets called and we
avoid (intermittent) shutdown tracebacks in the logs. This allows
upcoming integration tests that spawn a separate browser instance to
reuse this pattern to cleanly end the test.
Given that we integrate the `closeSinglePage` code from #17962 for this
patch, @calixteman is credited as the co-author.
Co-authored-by: Calixte Denizet <calixte.denizet@gmail.com>
This commit fixes the following log line that currently shows up for
every type of tests that involve a browser:
`System addon update list error SyntaxError: XMLHttpRequest.open:
'http://%(server)s/dummy-system-addons.xml' is not a valid URL.`
If Firefox is in testing mode, the system addons update URL is
configured to a dummy URL so that it can't actually update (see for
example the same value in Marionette at
https://searchfox.org/mozilla-central/source/testing/marionette/client/marionette_driver/geckoinstance.py#109),
but this doesn't stop Firefox from trying to update, and when it does
it logs this line because the URL is obviously invalid.
Hence this patch which disables system addon updates altogether so
Firefox doesn't attempt to use the dummy URL anymore. The browser
updates are all managed by Puppeteer, and regular updates have already
been disabled too (see
6937a76f0a/packages/browsers/src/browser-data/firefox.ts (L302-L303)).
Allow apps with supportsIntegratedFind to better monitor the find state.
A recognized use case is the Firefox findbar, its "not found" sound must
consider `entireWord` and only make noise when it is off.
See related implementation in
https://hg.mozilla.org/mozilla-central/rev/16b902cbcf26
This change can help if we have to move the implementation from cpp to jsm.
