Fix auto-rotate for printing landscape PDFs where the first page is also landscape

[escapewindow]

The current behavior for getPagesOverview assumes we want to only
auto-rotate if:

• enablePrintAutoRotate is true
• isFirstPagePortrait !== isPortraitOrientation(size)

This second check is what is breaking #9297. The two PDFs linked have a
landscape orientation first page, as well as subsequent pages. Since
false === false, we print portrait.

Let's drop the comparison with isFirstPagePortrait, and print
landscape if !isPortraitOrientation(size).

Fixes #9297.

[escapewindow]

Per #8043 (comment) , this seems to be a fix for #2851 as well. I don't know why we only rotate pages if the first page is portrait, introduced in #8043 ...

[escapewindow]

@timvandermeij do you have any guidance here?

I'm guessing that either we want this fix, or we want to keep the old behavior, and fix landscape printing some other way. If the latter is true, I'm curious if we have any preferred workflows (another pref? change enablePrintAutoRotate to an enum?)

[Yay295]

Looking at #8043 I feel like there could be some more changes made here.

1. isPortraitOrientation() was previously only called in two places. Now that you've removed one of them, it could be inlined.
2. The description for enablePrintAutoRotate in extensions/chromium/preferences_schema.json and web/pdf_viewer.js should be updated since it's not "automatic rotation of pages whose orientation differ from the first page" anymore.

I also think enablePrintAutoRotate should be true by default, but that's a different discussion.

[escapewindow]

The current behavior for getPagesOverview assumes we want to only
auto-rotate if:

* `enablePrintAutoRotate` is `true`

* `isFirstPagePortrait !== isPortraitOrientation(size)`

This second check is what is breaking #9297. The two PDFs linked have a
landscape orientation first page, as well as subsequent pages. Since
false === false, we print portrait.

Let's drop the comparison with isFirstPagePortrait, and print
landscape if !isPortraitOrientation(size).

Ideally, we'd replace the second check with a comparison against the printer page orientation. We may be able to get that information via print preview support.

[escapewindow]

Looking at #8043 I feel like there could be some more changes made here.

1. `isPortraitOrientation()` was previously only called in two places. Now that you've removed one of them, it could be inlined.

Hm, is that true?

(3.8.0:3.7.4:2.7.17) akimoz: ~/src/pdf/pdf.js (issue9297) [09:38:58]
10234$ rg -l isPortraitOrientation
web/pdf_document_properties.js
web/base_viewer.js
web/ui_utils.js
test/unit/ui_utils_spec.js

2. The description for `enablePrintAutoRotate` in `extensions/chromium/preferences_schema.json` and `web/pdf_viewer.js` should be updated since it's not "automatic rotation of pages **whose orientation differ from the first page**" anymore.

Makes sense. Done.

I also think enablePrintAutoRotate should be true by default, but that's a different discussion.

I'm leaning that way, but I'm not sure if there are reasons for the current behavior that I'm not aware of.

[Yay295]

Oh, you're right about isPortraitOrientation(). I was only looking at the changes in #8043 and didn't check that it had been used elsewhere since then.

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/f7647a5f08ba39d/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/f7647a5f08ba39d/output.txt

Total script time: 3.33 mins

Published

• Viewer: http://54.67.70.0:8877/f7647a5f08ba39d/web/viewer.html
• Viewer (ES5): http://54.67.70.0:8877/f7647a5f08ba39d/es5/web/viewer.html

[timvandermeij]

Thank you for improving this!

[escapewindow]

Thank you for the review!

[ilium007]

How can I patch my Firefox installs on OSX?

[timvandermeij]

This will be merged soon into Firefox Nightly and after that follow the normal Firefox release schedule.