1874947 - The usePageRuleSizeAsPaperSize internal print-setting only works for page sizes in unnamed page rules

Status: RESOLVED DUPLICATE of bug 1868708

(Core :: Printing: Output, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

While getting the WPT in https://phabricator.services.mozilla.com/D198195 to pass locally, AlaskanEmily and I discovered that our internal usePageRuleSizeAsPaperSize mechanism seems to only work if the page size is specified in an unnamed @page rule.

I'll post a testcase/reference case which also function as a WPT.

STR:

1. Load attached testcase.
2. Print-preview and select the Save-to-PDF backend.

EXPECTED RESULTS:
Firefox should preview the paper as being 2in by 3in.

ACTUAL RESULTS:
Firefox previews the paper as US Letter or whatever your default size is (and places the 2inx3in page onto that).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: reference case 1

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot comparing testcase (left) vs reference case (right)

Here's a screenshot showing the STR with testcase vs. reference case.

The main issue is the paper size difference; but one other issue that's also shown here: we allow orientation to be manually chosen for the testcase but not the reference case too, which is also inconsistent & probably wrong.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: patch to add WPT test

Here's a patch with attached testcase/reference case (with minor tweaks to fix some nits that I noticed after posting them). We can land this here when this bug is eventually fixed.

Chrome passes this WPT. Firefox fails with:

FAIL /css/css-page/page-name-001-print.html - Testing http://web-platform.test:8000/css/css-page/page-name-001-print.html == http://web-platform.test:8000/css/css-page/page-name-001-ref.html
Got different page sizes; test is 288x480px, ref is 192x288px

Comment 4

[Daniel Holbert [:dholbert]]

This is a recent regression, from bug 1869217.

Bug 1869217 doesn't have a lot of context, but it superficially sounds like this new behavior is ~expected based on the title of that bug. But I think it's probably not exactly what we wanted to achieve there?

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1869217

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 9373121 [details] [diff] [review]
patch to add WPT test

I landed a patch with a WPT for this in bug 1875186. That makes the patch attachment here no longer necessary, since the test now lives in-tree.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1869217

Comment 8

[Jonathan Watt [:jwatt]]

I'll get a patch up for this before code freeze.

Comment 9

[Donal Meehan [:dmeehan]]

This wont make a Fx122 release.
Fx123 is in the last week of beta, and goes to release candidate next week.
A patch would need to land by before the end of this week to be considered for uplift to beta.

Comment 10

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

:jwatt is this gonna ride the 125 trains?

Comment 11

[Jonathan Watt [:jwatt]]

I don't think this is really a regression.

Basically this is an instance of bug 1868708. Right now our @page code only supports setting the default sheet size. It always has, but with quirk that we would use the first page's @page page size as the default page size (contrary to what makes sense, IMO, and contrary to what other browsers do). The attached testcase used to work because of that (because it was setting the default size, not because we were correctly supporting and setting the size of the first sheet as an individual sheet, which is bug 1868708).

Comment 12

[Jonathan Watt [:jwatt]]

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

The main issue is the paper size difference; but one other issue that's also shown here: we allow orientation to be manually chosen for the testcase but not the reference case too, which is also inconsistent & probably wrong.

That's just logical fallout from the fact that the test does not set the sheet size (so the UI is not disabled), but the reference does (so the UI is disabled).

Comment 13

[Jonathan Watt [:jwatt]]

(In reply to Dianna Smith [:diannaS] from comment #10)

:jwatt is this gonna ride the 125 trains?

No. We should probably just dupe this to bug 1868708 IMO. I can try to get my patch for that finished off for the next cycle, but it isn't going to be finished for Wednesday.