Add Promise.withResolvers polyfill for latest PDF.js version #8199
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
toofar
changed the title
toofar
force-pushed
the
feat/8170_pdfjs_withresolve
branch
from
99e1d9d
to
a857edb
Compare
It's needed for both the main page (which we generate using jinja) and the worker script. Closes: #8170
This installs pdf.js in a selection of CI jobs. Previously the PDF.js tests (in qutescheme.feature) were skipped in CI because it wasn't installed anywhere. There has been a couple of recent cases where pdf.js started depending on javascript features that are too new for even the most recent QtWebEngine to support. The aim of this commit is to catch that case. This doesn't add coverage for older webengine releases. This also incidentally updates the ace editor in these test jobs, since that is also updated by default by the update_3rdparty script. Hopefully that doesn't cause issues. The reasoning for installing on each type of job: *ubuntu jobs*: not installed - while our main test runs are on ubuntu there is an upstream issue where many assets used by pdf.js (like icons used in the toolbar) aren't packaged, see #7904. This causes warning messages because assets requested via qutescheme can't be found, which causes the tests to fail. We could a) install pdf.js from source instead of using the ubuntu one b) ignore the warning logs c) skip this environment and rely on tests elsewhere. I've chosen to do (c). I don't see a huge benefit in testing pdf.js across multiple environments if we aren't using it installed from the OS anyway. We could install from source but currently all the Qt < 6.5 tests are failing from some other JS error, and I think fixing that is out of scope of this issue. *docker Qt6*: installed - the archlinux pdfjs package works fine and we are only testing the most recent Qt versions because arch users are expected to stay up to date. *docker Qt5*: not installed - doesn't support JS features required by PDF.js. I guess we could install the legacy build from source here. I'm mostly worried about catching new breakages for this commit though *windows*: installed - we install pdf.js from source when making a release so it would be nice to do that in tests too. *macos*: not installed - the tests that were catching the breakages are end2end tests which we don't run on mac. And I think there was an error from the :versions tests here, don't remember. *bleeding edge*: installed - from source pdf.js tests fail on Qt < 6.5 with `Uncaught TypeError: Cannot read properties of null (reading 'mainContainer')` The `TestPDFJSVersion.test_real_file` unit tests currently fails because `version._pdfjs_version()` returns `unknown (bundled)`, not sure why. I think this is pre-existing and it also wasn't being run on CI.
I'm trying to update pdf.js in the bleeding edge CI jobs. It complains
that either it can't find PyQt or it can't find yaml depending on how I
invoke tox. Joy. Since dict stuff isn't run by default in this script
hopefully that is the only broken import path and moving it into the
function lets the pdfjs (and ace) bit of the script work fine.
Actually, looking at the stack traces below, both of them are from dict
related code!
tox exec -re bleeding -- python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 15, in <module>
from qutebrowser.qt.core import pyqtSignal, pyqtBoundSignal, QObject
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/core.py", line 17, in <module>
machinery.init_implicit()
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/machinery.py", line 278, in init_implicit
raise NoWrapperAvailableError(info)
qutebrowser.qt.machinery.NoWrapperAvailableError: No Qt wrapper was importable.
python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 17, in <module>
from qutebrowser.utils import usertypes, log
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/usertypes.py", line 16, in <module>
from qutebrowser.utils import log, qtutils, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/qtutils.py", line 39, in <module>
from qutebrowser.utils import usertypes, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/utils.py", line 29, in <module>
import yaml
ModuleNotFoundError: No module named 'yaml'
As of at the very least the latest version the version string looks
like:
const pdfjsVersion = "4.2.67";
So change the regex to allow double quotes too.
toofar
force-pushed
the
feat/8170_pdfjs_withresolve
branch
from
a857edb
to
e86ef0b
Compare
|
Looks great, thanks! |
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.
Promise.withResolversfor recent versions of PDF.js.update_3rdparty.pyto only import stuff from qutebrowser if needed, due to some weirdness trying to run it in the bleeding edge test setupI tested the docker container locally and the relevant tests passed.
Closes: #8170