Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure that the viewer telemetry reporting, and fallback code, runs in development mode and GENERIC builds

Browse Source 
While only the `MOZCENTRAL` builds will actually do anything meaningful with the telemetry data, none of the code in question actually runs *at all* in e.g. development mode.[1]
This seems bad since it essentially means that this code is completely untested, despite being quite important for the built-in Firefox PDF viewer, and this thus ought to be fixed.

In this case, the explanation for the current state of the code should be "for historical reasons". Before the viewer was split into the current components and before the pre-processor was improved, back when all code resided in the `web/viewer.js` file, the telemetry reporting was done with *direct* `FirefoxCom` calls. However, with the dummy `DefaultExternalServices.reportTelemetry` method there's nothing actually preventing attempting to report telemetry in any type of build.

NOTE: By running this code in GENERIC builds as well, in addition to just locally, the *viewer* part of telemetry reporting becomes tested e.g. in preview builds too which should help with reviewing.

---
[1] When fixing bug 1606566, I had to edit the relevant `PDFJSDev` checks to be able to actually test the changes locally.
This commit is contained in:
Jonas Jenwald 2020-01-20 11:37:54 +01:00
parent ccf327538b
commit f171d99799
1 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
web/app.js
View File

@ -850,8 +850,8 @@ const PDFViewerApplication = {
fallback(featureId) {
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("FIREFOX || MOZCENTRAL")
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
// Only trigger the fallback once so we don't spam the user with messages
// for one PDF.
@ -1325,8 +1325,8 @@ const PDFViewerApplication = {
}
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("FIREFOX || MOZCENTRAL")
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
// Telemetry labels must be C++ variable friendly.
let versionId = "other";
@ -1556,8 +1556,8 @@ const PDFViewerApplication = {
printService.layout();
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("FIREFOX || MOZCENTRAL")
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
this.externalServices.reportTelemetry({
type: "print",
@ -1870,8 +1870,8 @@ function webViewerInitialized() {
}
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("FIREFOX || MOZCENTRAL") &&
(typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")) &&
!PDFViewerApplication.supportsDocumentFonts
) {
AppOptions.set("disableFontFace", true);
@ -1998,8 +1998,8 @@ function webViewerPageRendered(evt) {
}
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("FIREFOX || MOZCENTRAL")
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
PDFViewerApplication.externalServices.reportTelemetry({
type: "pageInfo",