Closed Bug 1833244 Opened 1 year ago Closed 10 months ago

Support `page-orientation` rotation on PrintedSheetFrame for pages-per-sheet = 1

Categories

(Core :: Printing: Setup, enhancement, P3)

Product:
Core
Component:
Printing: Setup
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
118 Branch
Tracking Flags:
Tracking Status
firefox118 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

Bug 1833244 p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert
48 bytes, text/x-phabricator-request
Details | Review
Bug 1833244 p2. Implement sheet orientation switching/rotation for `page-orientation`. r=alaskanemily,dholbert
48 bytes, text/x-phabricator-request
Details | Review

The patches for this are done, but will land after the patches for bug 1810750.

I'm just putting this up to make it available for the curious. This doesn't
actually build right now since I haven't fixed it up yet after moving bits of
the code to earlier patches that I'm actively working on to make reviews
easier.

Depends on D178440

Attachment #9334815 - Attachment is obsolete: true

OS print drivers/devices know nothing about page dimensions unless we tell
them. Previously, the physical page dimensions (including orientation) have
always been the same, so communicating their dimensions once at the start of
a print has been enough. In preparation for supporting different "physical"
page dimensions (in the immediate future only different page orientations) when
we save to PDF, we need to have the infrastructure to pass dimensions through
on a page-by-page basis. This patch adds that.

None of the PrintTarget subclasses do anything with this extra information yet,
but in a follow-up patch PrintTargetPDF will use this information to create
PDFs with mixed page orientations.

Attachment #9336516 - Attachment description: Bug 1833244 p1. Pass the print sheet size to PrintTarget::BeginPage overrides. r=dholbert → Bug 1833244 p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert

Where supported (print preview and print-to-PDF), this implements changing the
orientation and/or rotation of print sheets, as appropriate, in response to CSS
page-orientation. When supported we:

  • in the single page-per-sheet case, rotate the sheet in order to implement
    any page-orientation rotation on the sheet. Rotating the sheet is necessary
    so that the pages in the PDF files that we output are correct.

  • in the multiple pages-per-sheet case, we already rotate individual pages in
    their grid cell. This change keeps such pages rotated, as appropriate, but
    augments that behavior by switching the orientation of the sheet (based on
    the first page on the sheet) if necessary to best place the page to make
    maximum use of the space.

The following 'data' URI may be useful for testing. It makes it easy to switch which page is given the 'page-orientation', which helps test what happens when the rotated page is the first/not-first page on a sheet, both when the visual orientation of the sheet and page should be orthogonal (for first page at least), that is 2 pages-per-sheet, and when its not, as in 4 pages per sheet.

data:text/html,<title>hi</title><style>html, body, div { box-sizing: border-box; width:100%; height:100%; margin: 0; } div { border: 2px dashed red; } @page p2 { page-orientation: rotate-right; } </style><body><div style="page: p1">p1</div><div style="page: p2">p2</div><div style="page: p3">p3</div><div style="page: p4">p4</div><div style="page: p5">p5</div>

One thing to note is that in print preview sometimes at higher scaling factors the red border can disappear, but in the actual saved PDF or print output where the resolution is higher, the still appear.

Keywords: leave-open
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/autoland/rev/9fce1abd1c50
p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert

Backed out for causing build bustages on nsDeviceContextAndroid.h.

[task 2023-05-31T13:21:43.687Z] 13:21:43     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/xpcom/components'
[task 2023-05-31T13:21:43.690Z] 13:21:43     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ --target=x86_64-linux-android21 -o StaticComponents.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/xpcom/components -I/builds/worker/workspace/obj-build/xpcom/components -I/builds/worker/workspace/obj-build/xpcom -I/builds/worker/checkouts/gecko/xpcom/base -I/builds/worker/checkouts/gecko/xpcom/build -I/builds/worker/checkouts/gecko/xpcom/ds -I/builds/worker/checkouts/gecko/chrome -I/builds/worker/checkouts/gecko/js/xpconnect/loader -I/builds/worker/checkouts/gecko/layout/build -I/builds/worker/checkouts/gecko/modules/libjar -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h --sysroot=/builds/worker/fetches/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/builds/worker/fetches/android-ndk/toolchains/llvm/prebuilt/linux-x86_64 -fno-sized-deallocation -fno-aligned-new -fno-short-enums -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pipe -gdwarf-4 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Oz -mno-outline -fno-omit-frame-pointer -funwind-tables -Werror -Wall -Wbitfield-enum-conversion -Wdeprecated-this-capture -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wenum-compare-conditional -Wenum-float-conversion -Wno-error=deprecated -Wno-error=deprecated-anon-enum-enum-conversion -Wno-error=deprecated-enum-enum-conversion -Wno-error=deprecated-pragma -Wno-error=deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wformat -Wformat-security -Wno-psabi -Wthread-safety -Wno-error=builtin-macro-redefined -Wno-unknown-warning-option -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/StaticComponents.o.pp   StaticComponents.cpp
[task 2023-05-31T13:21:43.690Z] 13:21:43     INFO -  In file included from StaticComponents.cpp:283:
[task 2023-05-31T13:21:43.690Z] 13:21:43    ERROR -  /builds/worker/checkouts/gecko/xpcom/components/../../widget/android/nsDeviceContextAndroid.h:26:14: error: 'BeginPage' marked 'override' but does not override any member functions
[task 2023-05-31T13:21:43.690Z] 13:21:43     INFO -    NS_IMETHOD BeginPage() override { return NS_OK; }
[task 2023-05-31T13:21:43.690Z] 13:21:43     INFO -               ^
[task 2023-05-31T13:21:43.690Z] 13:21:43    ERROR -  /builds/worker/checkouts/gecko/xpcom/components/../../widget/android/nsDeviceContextAndroid.h:12:7: error: abstract class is marked 'final' [-Werror,-Wabstract-final-class]
[task 2023-05-31T13:21:43.690Z] 13:21:43     INFO -  class nsDeviceContextSpecAndroid final : public nsIDeviceContextSpec {
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -        ^
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -  /builds/worker/workspace/obj-build/dist/include/nsIDeviceContextSpec.h:89:14: note: unimplemented pure virtual method 'BeginPage' in 'nsDeviceContextSpecAndroid'
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -    NS_IMETHOD BeginPage(const IntSize& aSizeInPoints) = 0;
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -               ^
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -  In file included from StaticComponents.cpp:283:
[task 2023-05-31T13:21:43.691Z] 13:21:43    ERROR -  /builds/worker/checkouts/gecko/xpcom/components/../../widget/android/nsDeviceContextAndroid.h:26:14: error: 'nsDeviceContextSpecAndroid::BeginPage' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -    NS_IMETHOD BeginPage() override { return NS_OK; }
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -               ^
[task 2023-05-31T13:21:43.691Z] 13:21:43     INFO -  /builds/worker/workspace/obj-build/dist/include/nsIDeviceContextSpec.h:89:14: note: hidden overloaded virtual function 'nsIDeviceContextSpec::BeginPage' declared here: different number of parameters (1 vs 0)
[task 2023-05-31T13:21:43.692Z] 13:21:43     INFO -    NS_IMETHOD BeginPage(const IntSize& aSizeInPoints) = 0;
[task 2023-05-31T13:21:43.692Z] 13:21:43     INFO -               ^
[task 2023-05-31T13:21:43.692Z] 13:21:43    ERROR -  StaticComponents.cpp:10604:53: error: allocating an object of abstract class type 'nsDeviceContextSpecAndroid'
[task 2023-05-31T13:21:43.692Z] 13:21:43     INFO -        RefPtr<nsDeviceContextSpecAndroid> inst = new nsDeviceContextSpecAndroid();
[task 2023-05-31T13:21:43.692Z] 13:21:43     INFO -                                                      ^
[task 2023-05-31T13:21:43.692Z] 13:21:43     INFO -  4 errors generated.
[task 2023-05-31T13:21:43.692Z] 13:21:43    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:668: StaticComponents.o] Error 1
[task 2023-05-31T13:21:43.693Z] 13:21:43     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/xpcom/components'
[task 2023-05-31T13:21:43.693Z] 13:21:43     INFO -  gmake[4]: Target 'target-objects' not remade because of errors.
[task 2023-05-31T13:21:43.693Z] 13:21:43    ERROR -  gmake[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: xpcom/components/target-objects] Error 2
[task 2023-05-31T13:21:43.693Z] 13:21:43     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/mfbt/tests'
Flags: needinfo?(jwatt)

This disables the pref for now so that the following patch doesn't change any
behavior. We'll re-enable the pref shortly.

Attachment #9336551 - Attachment description: Bug 1833244 p2. Implement sheet orientation switching/rotation for `page-orientation`. r=alaskanemily,dholbert → Bug 1833244 p3. Implement sheet orientation switching/rotation for `page-orientation`. r=alaskanemily,dholbert
Attachment #9337295 - Attachment description: Bug 1833244 p2. Disable pref layout.css.page-orientation.enabled. r=dholbert → Bug 1833244 p1. Disable pref layout.css.page-orientation.enabled. r=dholbert
Attachment #9336516 - Attachment description: Bug 1833244 p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert → Bug 1833244 p2. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert
Pushed by jwatt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90a5bbba7b9c
p1. Disable pref layout.css.page-orientation.enabled. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/4576af83a4ec
p2. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/7bc8c25b2935
p3. Implement sheet orientation switching/rotation for `page-orientation`. r=AlaskanEmily,dholbert

Backed out for causing multiple failures.

  • Push with failures - Bp-nu bustages
  • Failure Log
  • Failure line: /builds/worker/checkouts/gecko/layout/generic/PrintedSheetFrame.cpp:269:21: error: no member named 'layout_css_page_orientation_enabled' in namespace 'mozilla::StaticPrefs'
  • Push with failures - reftests
  • Failure Log
  • Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/pagination/inline-block-slice-5b.html == layout/reftests/pagination/inline-block-slice-5b-ref.html | image comparison, max difference: 102, number of differing pixels: 49

The pref disabling depends on bug 1836623 to avoid the devtools test failure (I could alternatively remove the definition of the pref in devtools, but make the test robust to pref disabling in Nightly).

Flags: needinfo?(jwatt)

(In reply to Jonathan Watt [:jwatt] from comment #11)

I could alternatively remove the definition of the pref in devtools, but make the test robust to pref disabling in Nightly).

I think that's the correct thing to do, when the pref is disabled in Nightly. ./mach devtools-css-db should do it automagically? (and would make sense to include in the patch that toggles the pref to be false)

Attachment #9336516 - Attachment description: Bug 1833244 p2. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert → Bug 1833244 p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert
Attachment #9337295 - Attachment is obsolete: true
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/autoland/rev/4d154b557e54
p1. Create infrastructure to pass page dimensions to PrintTarget::BeginPage. r=dholbert,geckoview-reviewers,jonalmeida
Attachment #9336551 - Attachment description: Bug 1833244 p3. Implement sheet orientation switching/rotation for `page-orientation`. r=alaskanemily,dholbert → Bug 1833244 p2. Implement sheet orientation switching/rotation for `page-orientation`. r=alaskanemily,dholbert
Attachment #9337336 - Attachment is obsolete: true
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/autoland/rev/bc3dcd234d91
p2. Implement sheet orientation switching/rotation for `page-orientation`. r=AlaskanEmily,dholbert

Backed out for causing multiple failures with page_size

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | layout/base/tests/chrome/test_printpreview.xhtml | Printing print_page_size3.html and print_page_size3_ref.html should produce the same results - got false, expected true

Wpt log: https://treeherder.mozilla.org/logviewer?job_id=426649882&repo=autoland
Fail line: TEST-UNEXPECTED-ERROR | /css/printing/page-size-009-print.html | Testing http://web-platform.test:8000/css/printing/page-size-009-print.html == http://web-platform.test:8000/css/printing/page-size-009-print-ref.html

Flags: needinfo?(jwatt)
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/autoland/rev/6db2de19f043
p2. Implement sheet orientation switching/rotation for `page-orientation`. r=AlaskanEmily,dholbert

Backed out for causing wp failures on page-size-006-print.html.

''' INFO '''

Reftest analyzer says "Images are identical".

''' INFO '''

Flags: needinfo?(jwatt)
Flags: needinfo?(jwatt)
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/autoland/rev/e5eaba75408a
p2. Implement sheet orientation switching/rotation for `page-orientation`. r=AlaskanEmily,dholbert
Flags: needinfo?(jwatt)

jwatt: I think we can close this bug at this point, right?

(It didn't auto-close when hitting central, since it's got the leave-open keyword from a few months back, but I think that's no longer needed?)

Flags: needinfo?(jwatt)
Regressions: 1849890
status-firefox118: --- → ?
Duplicate of this bug: 1810750

(In reply to Daniel Holbert [:dholbert] from comment #22)

jwatt: I think we can close this bug at this point, right?

Yeah, I was going to leave this open while I finish bug 1816554, then get back to wrapping up the tests here. I could open a separate bug for the tests for this bug though if you prefer.

(In reply to Jonathan Watt [:jwatt] from comment #24)

(In reply to Daniel Holbert [:dholbert] from comment #22)

jwatt: I think we can close this bug at this point, right?

Yeah, I was going to leave this open while I finish bug 1816554, then get back to wrapping up the tests here. I could open a separate bug for the tests for this bug though if you prefer.

Maybe a followup bug would be good, to cover that remaining test work? But yeah, whatever seems simplest/easiest on your end. I just want to be sure this bug ends up in a consistent state, with us being able to track which-patches-landed-in-which-releases in a coherent way.

(also: I'm adding test-related bug 1849648 as a dependency, since per its initial comment, it's got a connection here. But I think that's unrelated to the tests that you're talking about.)

Blocks: 1849648

The code landed in Firefox 118 which releases soon, so I'll close this so it is captured in the list of fixed bugs for 118.

Status: NEW → RESOLVED
Closed: 10 months ago
status-firefox118: ? → fixed
Depends on: 1833239, 1835447, 1835475
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
See Also: → 1833241
Blocks: 1869880
Flags: needinfo?(jwatt)
Regressions: 1882056
You need to log in before you can comment on or make changes to this bug.