864250 - Create a browser chrome test that checks the zoom level property of the object

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Cornel Ionce [:noni]]

STR:

Precondition: Preferences/Options >Applications - PDFs use "Preview in <Firefox>"

1. Open a pdf document
2. Zoom in/out using the +/- buttons
3. Zoom in/out using zoom picker
4. Zoom in/out using CTRL+/CTRL-
5. Zoom in/out using CTRL + mouse scroll

Expected Results:
Document zooms in/out properly.

Notes:
This test is available in MozTrap as test case #368.

Comment 1

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1

Test zooming the pdf using the +/- buttons, CTRL++, CTRL+- and the zoom picker.

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8509532 [details] [diff] [review]
v1

Review of attachment 8509532 [details] [diff] [review]:
-----------------------------------------------------------------

I may not be the best person to review that as I don't know the details of the pdfjs document, I'm hoping there are some events that can be used instead of timeouts to catch the zoom changes.

::: browser/extensions/pdfjs/test/browser_pdfjs_zoom.js
@@ +19,5 @@
> +      selector: "button#zoomOut",
> +      event: "click"
> +    },
> +    expectedZoom: -1, // -1 - zoom out
> +    message: "Zoomed in using the '-' (zoom out) button"

* Zoomed out

@@ +112,5 @@
> +      deferred.resolve();
> +    }
> +  }, 500);
> +
> +  return deferred.promise;

Is there no event we can wait for here?

@@ +115,5 @@
> +
> +  return deferred.promise;
> +}
> +
> +function zoomPDF(document, window, test, endCallback) {

No need to pass the test in here, just do let test = TESTS.shift() at the start.

@@ +177,5 @@
> +      deferred.resolve();
> +    }
> +  }, 500);
> +
> +  return deferred.promise;

Is there no event we can wait for here?

Comment 3

[Yury Delendik (:yury)]

Comment on attachment 8509532 [details] [diff] [review]
v1

Review of attachment 8509532 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with the changes below

::: browser/extensions/pdfjs/test/browser.ini
@@ +6,5 @@
>  skip-if = debug # bug 1058695
>  [browser_pdfjs_savedialog.js]
>  [browser_pdfjs_views.js]
>  skip-if = debug # bug 1058695
> +[browser_pdfjs_zoom.js]

I think you shall add `skip-if = debug # bug 1058695` for this entry as well

::: browser/extensions/pdfjs/test/browser_pdfjs_zoom.js
@@ +165,5 @@
> +    ok(false, "Test '" + test.message + "' failed with timeout.");
> +  });
> +}
> +
> +function waitForZoom(document) {

You can use 'pagerendered' event here. See example at http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/content/web/viewer.js#6375 (make sure e.detail.pageNumber equal to page 1).

Comment 4

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1.1

Updated based on review.

Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7d2847249da5

Comment 5

[Yury Delendik (:yury)]

Comment on attachment 8517298 [details] [diff] [review]
v1.1

Review of attachment 8517298 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with the event handler change

::: browser/extensions/pdfjs/test/browser_pdfjs_zoom.js
@@ +119,5 @@
> +function zoomPDF(document, window, test, endCallback) {
> +  var renderedPage;
> +
> +  document.addEventListener("pagerendered", function onPageRendered(e) {
> +    document.removeEventListener("pagerendered", onPageRendered, true);

Several pages might be rendered at the same time, e.g. when browser window is tall enough (and test pdf file has multiple pages). Check if rendered page is 1 before detaching the event handler.

Comment 6

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1.2

Updated based un Yury's review. 
Carrying over the r+

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8612d94d1d6e

Comment 7

[Mihaela Velimiroviciu (:mihaelav)]

(In reply to Mihaela Velimiroviciu [QA] (:mihaelav) from comment #6)
> try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8612d94d1d6e

Try is green.
Marking for checkin

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/68514a13df83

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/68514a13df83