Re-factor the default preferences generation to support build targets (PR 10548) #13117
Conversation
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/e36e2b95d6fa819/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/421440927affbaa/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/e36e2b95d6fa819/output.txt Total script time: 3.53 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/421440927affbaa/output.txt Total script time: 5.79 mins
|
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/a13b19cee79a272/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/a13b19cee79a272/output.txt Total script time: 4.06 mins Published |
Snuffleupagus
force-pushed
the
defaultPreferences-builds
branch
from
a606212
to
97c9b0f
Compare
Snuffleupagus
force-pushed
the
defaultPreferences-builds
branch
2 times, most recently
from
3836b49
to
df39def
Compare
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/c949aca1f82e1bb/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/7ab91f4dce459b5/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/c949aca1f82e1bb/output.txt Total script time: 1.75 mins
|
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/7ab91f4dce459b5/output.txt Total script time: 3.75 mins
|
Snuffleupagus
force-pushed
the
defaultPreferences-builds
branch
from
df39def
to
a13c533
Compare
… (PR 10548) Originally the default preferences where simply placed in a JSON-file, checked into the repository, which over time became impractical, annoying, and error-prone to maintain; please see PR 10548. While that improved the overall situation a fair bit, it however inherited one quite unfortunate property of the old JSON-based solution[1]: It's still not possible for *different* build targets to specify their *own* default preference values. With some preferences, such as e.g. `enableScripting`, it's not inconceivable that you'd want to (at least) support build-specific default preference values. Currently that's not really possible, which is why this PR re-factors the default preferences generation to support this. --- [1] This fact isn't really clear from the `AppOptions` implementation, unless you're familiar with the `gulpfile.js` code, which could lead to some confusion for those new to this part of the code-base.
Snuffleupagus
force-pushed
the
defaultPreferences-builds
branch
from
a13c533
to
7d64d5a
Compare
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/3c638d7ece432b9/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/2ef1e4b033b25fc/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/3c638d7ece432b9/output.txt Total script time: 3.69 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/2ef1e4b033b25fc/output.txt Total script time: 5.92 mins
|
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/53257c64e818ec4/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/872432163a88bb8/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/53257c64e818ec4/output.txt Total script time: 3.09 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/872432163a88bb8/output.txt Total script time: 4.76 mins
|
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bd23d8a7a5826aa/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/bd23d8a7a5826aa/output.txt Total script time: 4.22 mins Published |
…Preferences`, to a helper function Currently there's a lot of duplication in the `buildLib` and `buildDefaultPreferences` functions, which seem quite unfortunate. Hence this patch extracts the common functionality in a new `buildLibHelper` function instead.
…etry" preference With the changes made in the previous patch, we can now list "disableTelemetry" in the `AppOptions` only for the `CHROME`-builds and thus remove the special-casing in the `checkChromePreferencesFile` helper function.
…ns`-methods Given how the compatibility-values are being handled, it's not actually possible to override a *truthy* default-value with a *falsy* compatibility-value. This is a simple oversight on my part, and with modern ECMAScript features this is very easy to support.
Snuffleupagus
force-pushed
the
defaultPreferences-builds
branch
from
7d64d5a
to
7bcfabc
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/39a915fc2925dd7/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/39a915fc2925dd7/output.txt Total script time: 4.23 mins Published |
|
Looks like a really good idea; thank you for doing this! |
As always, thank you for review/landing my patches :-) This now leaves only the https://github.com/mozilla/pdf.js/blob/master/extensions/chromium/preferences_schema.json file as a slight annoyance, since it still requires manual updates whenever the Looking at https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm/ I'm also starting to wonder if the Chromium extension is still being maintained, since the last update is listed as "13 september 2019" which is now a year and a half ago. |
|
@Rob--W Do you have more information regarding the Chrome extension? It seems to very out of date; is it still maintained? |
|
I still support it, but an update is indeed overdue. Due to the age and updates (appearance, experimental JS support), I need to do some testing and perhaps an update of defaults (e.g. enabling dark theme by default) to make sure that users still recognize the UI. |
Just a FYI: Scripting will be enabled by default for all user starting with Firefox 88; please see https://bugzilla.mozilla.org/show_bug.cgi?id=1699219
That should now be easy to change for the Chromium extension, by updating Line 159 in d426ffd value: typeof PDFJSDev !== "undefined" && PDFJSDev.test("CHROME") ? 2 : 0, |
Originally the default preferences where simply placed in a JSON-file, checked into the repository, which over time became impractical, annoying, and error-prone to maintain; please see PR #10548.
While that improved the overall situation a fair bit, it however inherited one quite unfortunate property of the old JSON-based solution[1]: It's still not possible for different build targets to specify their own default preference values.
With some preferences, such as e.g.
enableScripting, it's not inconceivable that you'd want to (at least) support build-specific default preference values. Currently that's not really possible, which is why this PR re-factors the default preferences generation to support this.Much smaller/simpler diff with https://github.com/mozilla/pdf.js/pull/13117/files?w=1
[1] This fact isn't really clear from the
AppOptionsimplementation, unless you're familiar with thegulpfile.jscode, which could lead to some confusion for those new to this part of the code-base.