Closed Bug 1414600 Opened 7 years ago Closed 4 years ago

viewport units incorrect in print

Categories

(Core :: CSS Parsing and Computation, defect, P2)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
81 Branch
Project Flags:
Webcompat Priority ?
Tracking Flags:
Tracking Status
firefox81 --- fixed

People

(Reporter: fantasai.bugs, Assigned: emilio)

References

Details

(Keywords: testcase, Whiteboard: [layout:print-triage:p2][print2020_v81], [wptsync upstream])

Attachments

(4 files)

patch for reftest
3.08 KB, text/plain
Details
Relevant frame tree bits
1.99 KB, text/plain
Details
Another testcase
245 bytes, text/html
Details
Bug 1414600 - Make media queries and viewport units in print be evaluated against the default page size minus margins. r=#style
47 bytes, text/x-phabricator-request
Details | Review
Attached file patch for reftest —
Viewport units are defined to be a percentage of the initial containing block. This seems to work fine in the case of screen media, but in print they are too big, probably because they're not subtracting out the page margin etc. Relevant code exists in CalcViewportUnitsScale in nsRuleNode.cpp and, I think, viewport_size in context.rs
Probably it should just use mPageSize when printing? This looks similar to bug 466559.
See Also: → 466559
Assignee: nobody → emilio
I was too eager to take this because I saw a patch similar to that made the reftest fail, but I didn't recall that reftest-print is not a thing anymore :) Thus, still looking at it, but probably not as easy...
Assignee: emilio → nobody
Err, pass I mean, of course
I suspect we need to substract the ::-moz-page margins, but not sure yet. Also, looks to me that viewport media queries and viewport units should agree, so there's probably still a bug in media queries, too...
So, I took a look at that, and we have a page that looks as follows: GetVisibleArea(): (204800, 148640) GetPageSize(): (48960, 63360) ::-moz-pagecontent margin (multiplied by two, to account for top / bottom, and left / right): (720, 1440) Root element frame: Block(html)(-1)@7fa8238beb28 {0,0,40320,15840} [state=0200160800d00210] [content=7fa8238b93a0] [sc=7fa824b89238]< GetPageSize() - margin: (47520, 60480) So even with the margins substracted, that doesn't reach the doc element frame. So there's extra margins or stuff like that that we don't account for, and I don't know off-hand where to look for them :)
(Just for reference)
Hmm, Boris pointed out on IRC we have prefs to configure page margins, which look relevant: https://searchfox.org/mozilla-central/rev/fe75164c55321e011d9d13b6d05c1e00d0a640be/layout/generic/nsSimplePageSequenceFrame.cpp#199-201
(In reply to Emilio Cobos Álvarez [:emilio] from comment #7) > Hmm, Boris pointed out on IRC we have prefs to configure page margins, which > look relevant: > > https://searchfox.org/mozilla-central/rev/ > fe75164c55321e011d9d13b6d05c1e00d0a640be/layout/generic/ > nsSimplePageSequenceFrame.cpp#199-201 So, cool part is that with these bits I'm pretty sure I can do the math to properly get the doc element containing block. The not-so-cool part is that nsIPrintSettings is an XPCOM interface that can be implemented in JS (and indeed is), so calling that off-main-thread would be a disaster...
Priority: -- → P3
I wonder whether we can grab the margin state during pagesequence frame construction, not reflow... or even print presshell init, since I guess we could have media queries based on viewport units that affect frame construction.
Attached file Another testcase —

This results in 2 pages in Firefox Print Preview vs 1 page in Chrome Print Preview.

This means any print regression tests using these units are also probably wrong... :(

I tend to think this is a Defect, not an Enhancement, and it seems like a compat issue we should try to address sooner rather than later.

Type: enhancement → defect
Webcompat Priority: --- → ?
Keywords: testcase
OS: Unspecified → All
Priority: P3 → P2
Hardware: Unspecified → All

This means any print regression tests using these units are also probably wrong... :(

Actually, it seems to be worse than I initially thought. In the "paged mode" we use for reftests (<html class="reftest-paged">) it seems 100vh is sized to the viewport of the window's viewport rather than the viewport associated with the page.
So it appears that vh/vw is completely useless for writing regression tests using "reftest-paged" since it gives different results than in actual Print/Print Preview.

Flags: needinfo?(emilio)
Whiteboard: [layout:print-triage:p2]
Assignee: nobody → emilio

For that, move the default page margin computation to nsPresContext.

See https://github.com/w3c/csswg-drafts/issues/5437 as to why doing this
and other alternatives.

Flags: needinfo?(emilio)
Whiteboard: [layout:print-triage:p2] → [layout:print-triage:p2][print2020_v81]
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b6d1532f6cd4 Make media queries and viewport units in print be evaluated against the default page size minus margins. r=dholbert,nordzilla
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/25087 for changes under testing/web-platform/tests
Whiteboard: [layout:print-triage:p2][print2020_v81] → [layout:print-triage:p2][print2020_v81], [wptsync upstream]

Backed out for reftest failures on mq_print_height.xhtml

backout: https://hg.mozilla.org/integration/autoland/rev/383d7d1d2d2f2b01b96181b9fec45a0cc3f89d65

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b6d1532f6cd4f3dabe20557fddcfb8595a6ba809&searchStr=reftest&selectedTaskRun=NY-FCxQMS3mN6U-z8xrTDQ.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313384067&repo=autoland&lineNumber=31756

[task 2020-08-19T01:38:33.450Z] 01:38:33 INFO - REFTEST TEST-START | layout/reftests/css-mediaqueries/mq_print_height.xhtml == layout/reftests/css-mediaqueries/mq_print-ref.xhtml
[task 2020-08-19T01:38:33.450Z] 01:38:33 INFO - REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/layout/reftests/css-mediaqueries/mq_print_height.xhtml | 5876 / 7234 (81%)
[task 2020-08-19T01:38:33.451Z] 01:38:33 INFO - REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/layout/reftests/css-mediaqueries/mq_print-ref.xhtml | 5876 / 7234 (81%)
[task 2020-08-19T01:38:33.451Z] 01:38:33 INFO - REFTEST INFO | REFTEST fuzzy test (0, 0) <= (255, 367) <= (8, 454)
[task 2020-08-19T01:38:33.454Z] 01:38:33 WARNING - REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/css-mediaqueries/mq_print_height.xhtml == layout/reftests/css-mediaqueries/mq_print-ref.xhtml | image comparison, max difference: 255, number of differing pixels: 367

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/18dbcec715c7 Make media queries and viewport units in print be evaluated against the default page size minus margins. r=dholbert,nordzilla
Upstream PR merged by moz-wptsync-bot

https://hg.mozilla.org/mozilla-central/rev/18dbcec715c7

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox81: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Regressions: 1660938
Regressions: 1663454
Regressions: 1814057
No longer regressions: 1814057
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: