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

[Firefox] Generate a PDF.js default-prefs file that can be used directly in mozilla-central (bug 1905864) #15209

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

Snuffleupagus
Copy link
Collaborator

No description provided.

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Jul 22, 2022

Please note: This will obviously require changes in the mozilla-central code as well, so needs to land simultaneously in GitHub/Bugzilla, however I'll probably need some guidance w.r.t. how the mozilla-central part should look to integrate correctly with the prefs-system.

The file that's generated looks like this:

/* Copyright 2023 Mozilla Foundation
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

// THIS FILE IS GENERATED AUTOMATICALLY, DO NOT EDIT MANUALLY!
//
// Any overrides should be placed in `PdfJsOverridePrefs.js`.

pref("pdfjs.annotationEditorMode", 0);
pref("pdfjs.annotationMode", 2);
pref("pdfjs.cursorToolOnLoad", 0);
pref("pdfjs.defaultZoomDelay", 400);
pref("pdfjs.defaultZoomValue", "");
pref("pdfjs.disableAutoFetch", false);
pref("pdfjs.disableFontFace", false);
pref("pdfjs.disablePageLabels", false);
pref("pdfjs.disableRange", false);
pref("pdfjs.disableStream", false);
pref("pdfjs.enableHWA", false);
pref("pdfjs.enableHighlightEditor", false);
pref("pdfjs.enableHighlightFloatingButton", false);
pref("pdfjs.enableML", false);
pref("pdfjs.enablePermissions", false);
pref("pdfjs.enablePrintAutoRotate", true);
pref("pdfjs.enableScripting", true);
pref("pdfjs.enableStampEditor", true);
pref("pdfjs.enableXfa", true);
pref("pdfjs.externalLinkTarget", 0);
pref("pdfjs.forcePageColors", false);
pref("pdfjs.highlightEditorColors", "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F");
pref("pdfjs.historyUpdateUrl", false);
pref("pdfjs.ignoreDestinationZoom", false);
pref("pdfjs.pageColorsBackground", "Canvas");
pref("pdfjs.pageColorsForeground", "CanvasText");
pref("pdfjs.pdfBugEnabled", false);
pref("pdfjs.scrollModeOnLoad", -1);
pref("pdfjs.sidebarViewOnLoad", -1);
pref("pdfjs.spreadModeOnLoad", -1);
pref("pdfjs.textLayerMode", 1);
pref("pdfjs.viewOnLoad", 0);

#include PdfJsOverridePrefs.js

@Snuffleupagus Snuffleupagus force-pushed the createDefaultPrefsFile branch 2 times, most recently from b89cd95 to 2668799 Compare July 22, 2022 15:24
@Snuffleupagus Snuffleupagus changed the title [Firefox] Generate a default-prefs file that can used directly in mozilla-central (bug 1780609) [Firefox] Generate a default-prefs file that can be used directly in mozilla-central (bug 1780609) Jul 22, 2022
@marco-c marco-c marked this pull request as ready for review November 30, 2022 14:05
@marco-c marco-c marked this pull request as draft November 30, 2022 14:07
@Snuffleupagus Snuffleupagus force-pushed the createDefaultPrefsFile branch 4 times, most recently from 873d4f8 to aee0da1 Compare March 30, 2023 16:08
@Snuffleupagus Snuffleupagus force-pushed the createDefaultPrefsFile branch 2 times, most recently from eb9b4a0 to 1e01b86 Compare October 8, 2023 12:13
@Snuffleupagus Snuffleupagus force-pushed the createDefaultPrefsFile branch 3 times, most recently from b830d16 to 7264592 Compare November 2, 2023 12:52
@calixteman
Copy link
Contributor

@Snuffleupagus what do you have to do on this PR to make it ready ?

@Snuffleupagus
Copy link
Collaborator Author

what do you have to do on this PR to make it ready ?

Unfortunately I don't know nearly enough about the Firefox preference-system to have any idea how the pref-file that's generated here would be integrated correctly on the mozilla-central side.

The idea behind this PR was that the new pref-file will (somehow?) be automatically loaded "directly" into the Firefox preference-system, such that we could get rid of e.g. this code: https://searchfox.org/mozilla-central/rev/5b037d9c6ecdb0729f39ad519f0b867d80a92aad/toolkit/components/pdfjs/content/PdfJs.sys.mjs#44-67

@calixteman
Copy link
Contributor

I don't know that much about pref stuff... but we can learn :)
After a quick search with searchfox I found this:
https://searchfox.org/mozilla-central/source/devtools/client/preferences/moz.build#7
So if we're lucky we could add a similar line in:
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/moz.build

@calixteman
Copy link
Contributor

calixteman commented Jun 28, 2024

I just tried to add JS_PREFERENCE_FILES in moz.build and it works !
We need to take care about the precedence of a pref defined in this new file and one defined in all.js or in geckoview-prefs.js.
That said, you should sort the prefs in the output and remove the multiple // THIS FILE IS GENERATED AUTOMATICALLY, DO NOT EDIT MANUALLY! which are, from my pov, useless.

Note: if you want to be able to run pdf.js in m-c you just have to do that in viewer.mjs:

@@ -1826,6 +1859,12 @@ class ExternalServices extends BaseExter
   async getNimbusExperimentData() {
     return null;
   }
+  async getGlobalEventNames() {
+    return null;//FirefoxCom.requestAsync("getGlobalEventNames", null);
+  }

@Snuffleupagus
Copy link
Collaborator Author

I just tried to add JS_PREFERENCE_FILES in moz.build and it works !

I can confirm that locally, thank you!

We need to take care about the precedence of a pref defined in this new file and one defined in all.js or in geckoview-prefs.js.

Yes, it seems (in my quick testing) that the new pref-file overrides any entries from those files which is obviously not good.

@calixteman
Copy link
Contributor

After having thought about that, here are few things:

  • I'd like we use the normal pref stuff so I'm really in favor of having this patch merged
  • to fix the precedence issue we can either fetch the pref files and generate one which takes into account what is already defined in the fetched ones, or we can generate the ifdef stuff based on the configuration we've here (e.g. we can add some options to be able to have the pref for nightly, beta, whatever, ...). It should be possible to generate different files for firefox/geckoview (see https://searchfox.org/mozilla-central/source/browser/branding/branding-common.mozbuild#10). @Snuffleupagus Any other ideas ?

@Snuffleupagus
Copy link
Collaborator Author

[...] It should be possible to generate different files for firefox/geckoview

Sure, that would probably be possible but it seems a little bit "messy" to me.

How about the following work-around instead, where we generate a default-prefs file and have an "override" file that lives in mozilla-central? Unless I'm messing up when testing locally, this (to my surprise) seems to work OK:

diff --git a/mobile/android/app/geckoview-prefs.js b/mobile/android/app/geckoview-prefs.js
--- a/mobile/android/app/geckoview-prefs.js
+++ b/mobile/android/app/geckoview-prefs.js
@@ -377,12 +377,6 @@ pref("network.protocol-handler.warn-exte
 // (bug 888268)
 pref("network.tickle-wifi.enabled", true);
 
-// Editing PDFs is not supported on mobile
-pref("pdfjs.annotationEditorMode", -1);
-
-// Enable the floating PDF.js toolbar on GeckoView (bug 1829366)
-pref("pdfjs.enableFloatingToolbar", true);
-
 // Try to convert PDFs sent as octet-stream (bug 1754499)
 pref("pdfjs.handleOctetStream", true);
 
diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -174,29 +174,6 @@ pref("browser.helperApps.deleteTempFileO
 
 pref("browser.triple_click_selects_paragraph", true);
 
-// Enable fillable forms in the PDF viewer.
-pref("pdfjs.annotationMode", 2);
-
-// Enable editing in the PDF viewer.
-pref("pdfjs.annotationEditorMode", 0);
-
-// Enable JavaScript support in the PDF viewer.
-pref("pdfjs.enableScripting", true);
-
-// Enable XFA form support in the PDF viewer.
-pref("pdfjs.enableXfa", true);
-
-// Enable adding an image in a pdf.
-pref("pdfjs.enableStampEditor", true);
-
-// Enable highlighting in a pdf.
-pref("pdfjs.enableHighlightEditor", true);
-#if defined(EARLY_BETA_OR_EARLIER)
-  pref("pdfjs.enableHighlightFloatingButton", true);
-#else
-  pref("pdfjs.enableHighlightFloatingButton", false);
-#endif
-
 // Disable support for MathML
 pref("mathml.disabled",    false);
 
diff --git a/toolkit/components/pdfjs/content/PdfJs.sys.mjs b/toolkit/components/pdfjs/content/PdfJs.sys.mjs
--- a/toolkit/components/pdfjs/content/PdfJs.sys.mjs
+++ b/toolkit/components/pdfjs/content/PdfJs.sys.mjs
@@ -36,35 +36,6 @@ XPCOMUtils.defineLazyServiceGetter(
   "@mozilla.org/uriloader/handler-service;1",
   "nsIHandlerService"
 );
-const lazy = {};
-ChromeUtils.defineESModuleGetters(lazy, {
-  PdfJsDefaultPreferences: "resource://pdf.js/PdfJsDefaultPreferences.sys.mjs",
-});
-
-function initializeDefaultPreferences() {
-  var defaultBranch = Services.prefs.getDefaultBranch("pdfjs.");
-  var defaultValue;
-  for (var key in lazy.PdfJsDefaultPreferences) {
-    // Skip prefs that are already defined, so we can enable/disable things
-    // in all.js.
-    let prefType = defaultBranch.getPrefType(key);
-    if (prefType !== Ci.nsIPrefBranch.PREF_INVALID) {
-      continue;
-    }
-    defaultValue = lazy.PdfJsDefaultPreferences[key];
-    switch (typeof defaultValue) {
-      case "boolean":
-        defaultBranch.setBoolPref(key, defaultValue);
-        break;
-      case "number":
-        defaultBranch.setIntPref(key, defaultValue);
-        break;
-      case "string":
-        defaultBranch.setCharPref(key, defaultValue);
-        break;
-    }
-  }
-}
 
 // We're supposed to get this type of thing from the OS, and generally we do.
 // But doing so is expensive, so on startup paths we can use this to make the
@@ -117,8 +88,6 @@ export var PdfJs = {
 
     // Listen for when a different pdf handler is chosen.
     Services.obs.addObserver(this, TOPIC_PDFJS_HANDLER_CHANGED);
-
-    initializeDefaultPreferences();
   },
 
   uninit: function uninit() {
diff --git a/toolkit/components/pdfjs/content/PdfJsDefaultPreferences.sys.mjs b/toolkit/components/pdfjs/content/PdfJsDefaultPreferences.sys.mjs
deleted file mode 100644
--- a/toolkit/components/pdfjs/content/PdfJsDefaultPreferences.sys.mjs
+++ /dev/null
@@ -1,53 +0,0 @@
-/* Copyright 2023 Mozilla Foundation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-//
-// THIS FILE IS GENERATED AUTOMATICALLY, DO NOT EDIT MANUALLY!
-//
-
-export const PdfJsDefaultPreferences = Object.freeze({
-  annotationEditorMode: 0,
-  annotationMode: 2,
-  cursorToolOnLoad: 0,
-  defaultZoomDelay: 400,
-  defaultZoomValue: "",
-  disablePageLabels: false,
-  enableHighlightEditor: false,
-  enableHighlightFloatingButton: false,
-  enableML: false,
-  enablePermissions: false,
-  enablePrintAutoRotate: true,
-  enableScripting: true,
-  enableStampEditor: true,
-  externalLinkTarget: 0,
-  highlightEditorColors: "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F",
-  historyUpdateUrl: false,
-  ignoreDestinationZoom: false,
-  forcePageColors: false,
-  pageColorsBackground: "Canvas",
-  pageColorsForeground: "CanvasText",
-  pdfBugEnabled: false,
-  sidebarViewOnLoad: -1,
-  scrollModeOnLoad: -1,
-  spreadModeOnLoad: -1,
-  textLayerMode: 1,
-  viewOnLoad: 0,
-  disableAutoFetch: false,
-  disableFontFace: false,
-  disableRange: false,
-  disableStream: false,
-  enableHWA: false,
-  enableXfa: true
-});
diff --git a/toolkit/components/pdfjs/content/PdfJsDefaultPrefs.js b/toolkit/components/pdfjs/content/PdfJsDefaultPrefs.js
new file mode 100644
--- /dev/null
+++ b/toolkit/components/pdfjs/content/PdfJsDefaultPrefs.js
@@ -0,0 +1,53 @@
+/* Copyright 2023 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// THIS FILE IS GENERATED AUTOMATICALLY, DO NOT EDIT MANUALLY!
+//
+// Any overrides should be placed in `PdfJsOverridePrefs.js`.
+
+pref("pdfjs.annotationEditorMode", 0);
+pref("pdfjs.annotationMode", 2);
+pref("pdfjs.cursorToolOnLoad", 0);
+pref("pdfjs.defaultZoomDelay", 400);
+pref("pdfjs.defaultZoomValue", "");
+pref("pdfjs.disableAutoFetch", false);
+pref("pdfjs.disableFontFace", false);
+pref("pdfjs.disablePageLabels", false);
+pref("pdfjs.disableRange", false);
+pref("pdfjs.disableStream", false);
+pref("pdfjs.enableHWA", false);
+pref("pdfjs.enableHighlightEditor", false);
+pref("pdfjs.enableHighlightFloatingButton", false);
+pref("pdfjs.enableML", false);
+pref("pdfjs.enablePermissions", false);
+pref("pdfjs.enablePrintAutoRotate", true);
+pref("pdfjs.enableScripting", true);
+pref("pdfjs.enableStampEditor", true);
+pref("pdfjs.enableXfa", true);
+pref("pdfjs.externalLinkTarget", 0);
+pref("pdfjs.forcePageColors", false);
+pref("pdfjs.highlightEditorColors", "yellow=#FFFF98,green=#53FFBC,blue=#80EBFF,pink=#FFCBE6,red=#FF4F5F");
+pref("pdfjs.historyUpdateUrl", false);
+pref("pdfjs.ignoreDestinationZoom", false);
+pref("pdfjs.pageColorsBackground", "Canvas");
+pref("pdfjs.pageColorsForeground", "CanvasText");
+pref("pdfjs.pdfBugEnabled", false);
+pref("pdfjs.scrollModeOnLoad", -1);
+pref("pdfjs.sidebarViewOnLoad", -1);
+pref("pdfjs.spreadModeOnLoad", -1);
+pref("pdfjs.textLayerMode", 1);
+pref("pdfjs.viewOnLoad", 0);
+
+#include PdfJsOverridePrefs.js
\ No newline at end of file
diff --git a/toolkit/components/pdfjs/content/PdfJsOverridePrefs.js b/toolkit/components/pdfjs/content/PdfJsOverridePrefs.js
new file mode 100644
--- /dev/null
+++ b/toolkit/components/pdfjs/content/PdfJsOverridePrefs.js
@@ -0,0 +1,49 @@
+/* Copyright 2023 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#if defined(ANDROID)
+
+  // Editing PDFs is not supported on mobile
+  pref("pdfjs.annotationEditorMode", -1);
+
+  // Enable the floating PDF.js toolbar on GeckoView (bug 1829366)
+  pref("pdfjs.enableFloatingToolbar", true);
+
+#else
+
+  // Enable fillable forms in the PDF viewer.
+  pref("pdfjs.annotationMode", 2);
+
+  // Enable editing in the PDF viewer.
+  pref("pdfjs.annotationEditorMode", 0);
+
+  // Enable JavaScript support in the PDF viewer.
+  pref("pdfjs.enableScripting", true);
+
+  // Enable XFA form support in the PDF viewer.
+  pref("pdfjs.enableXfa", true);
+
+  // Enable adding an image in a pdf.
+  pref("pdfjs.enableStampEditor", true);
+
+  // Enable highlighting in a pdf.
+  pref("pdfjs.enableHighlightEditor", true);
+  #if defined(EARLY_BETA_OR_EARLIER)
+    pref("pdfjs.enableHighlightFloatingButton", true);
+  #else
+    pref("pdfjs.enableHighlightFloatingButton", false);
+  #endif
+
+#endif
diff --git a/toolkit/components/pdfjs/jar.mn b/toolkit/components/pdfjs/jar.mn
--- a/toolkit/components/pdfjs/jar.mn
+++ b/toolkit/components/pdfjs/jar.mn
@@ -1,7 +1,6 @@
 pdfjs.jar:
 % resource pdf.js %content/
  content/PdfJs.sys.mjs (content/PdfJs.sys.mjs)
- content/PdfJsDefaultPreferences.sys.mjs (content/PdfJsDefaultPreferences.sys.mjs)
  content/PdfJsNetwork.sys.mjs (content/PdfJsNetwork.sys.mjs)
  content/PdfJsTelemetry.sys.mjs (content/PdfJsTelemetry.sys.mjs)
  content/PdfSandbox.sys.mjs (content/PdfSandbox.sys.mjs)
diff --git a/toolkit/components/pdfjs/moz.build b/toolkit/components/pdfjs/moz.build
--- a/toolkit/components/pdfjs/moz.build
+++ b/toolkit/components/pdfjs/moz.build
@@ -16,6 +16,10 @@ EXTRA_JS_MODULES += [
     "pdfjs.sys.mjs",
 ]
 
+JS_PREFERENCE_PP_FILES += [
+    "content/PdfJsDefaultPrefs.js",
+]
+
 XPCOM_MANIFESTS += [
     "components.conf",
 ]

@calixteman
Copy link
Contributor

Excellent idea: I like it.

@Snuffleupagus

This comment was marked as resolved.

@Snuffleupagus Snuffleupagus changed the title [Firefox] Generate a default-prefs file that can be used directly in mozilla-central (bug 1780609) [Firefox] Generate a PDF.js default-prefs file that can be used directly in mozilla-central (bug 1905864) Jul 2, 2024
@Snuffleupagus Snuffleupagus marked this pull request as ready for review July 2, 2024 10:44
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for improving this.

@Snuffleupagus Snuffleupagus merged commit 396231b into mozilla:master Jul 4, 2024
9 checks passed
@Snuffleupagus Snuffleupagus deleted the createDefaultPrefsFile branch July 4, 2024 09:06
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.

2 participants