Closed Bug 1671036 Opened 4 years ago Closed 4 years ago

Page-margin settings aren't reliably respected in test_printpreview.xhtml

Categories

(Core :: Print Preview, defect)

defect

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: dholbert, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Whiteboard: [print2020_v83])

Attachments

(2 files)

I'm adding new tests in bug 1631452 which depend on the ability to set the page-margins to 0.

However, my Try runs have revealed that on some Windows platforms (Win10 x64 debug and ASAN in particular), we disregard the requested print settings' margin choices, some significant fraction of the time (in maybe 10% to 30% of test runs).

Sample try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1972464f58b6ad22882b0fa5f04cc7fa24631f39

In the test failures that mention "printpreview_pps" helper-files, this bug here is the root issue. If you look at the test-failures' data-URI screenshots in the log, you can see that the tests are being rendered with page-margins, even though the tests use zero page-margins.

(I'll post a PoC test-patch here that's sufficient to demonstrate the bug, independent of bug 1631452's changes.)

Attached patch patch-to-demonstrate-bug.patch (deleted) — Splinter Review

Here's a patch that adds a simple test at the end of test_printpreview.xhtml's main helper-file.

The test just loads an HTML file in two configurations:

  • with margins set to 0
  • with margins set to their default values
    ...and it asserts that the resulting snapshots should not match.

However, this test fails (i.e. the screenshots do match) a concerning percentage of the time.

Here's a Try run with the attached patch applied on top of mozilla-central:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=52cc2905c3f8c9859970b58b94334f4aaf72f8b2

This Try run currently has 11 instances of this test-job on Windows 10 x64, asan and debug. For both of these platforms, 4 of the 11 test runs had this problem and fail the included test.

(Unfortunately you can't visualize the test-failures in comment 1's try run, since I think test_printpreview.xhtml only dumps a data URI when there's a mismatch. And these failures in comment 1 are from an unexpected match.

However, you can visualize the test failure in the pages-per-sheet Try run in comment 0. The test failures there do include data URIs because they are mismatches. From those snapshots, it's clear that we're including page margins when we should not be.)

Some things that I tested and ruled out as possible workarounds here:
(1) It doesn't matter whether you set the margins via settings.marginTop = 0 etc. vs. if you use settings.honorPageRuleMargins = true and @page { margin: 0 }. This problem still happens either way.

(2) It doesn't seem to matter whether the HTML testcases here are external files vs. data URIs. (I ran a try run using data URIs and still hit the issue.)

I'm guessing this is just a race condition of some sort, though I'm not sure if it's a race condition in the test vs. in our actual print pipeline.

Emilio, I'm wondering if you might have cycles to take a look here, or if you know this is a known issue, since you've been working in this area & done some work on this test recently? Hopefully the attached patch is enough for you to go on. (Note that the patch is based on current mozilla-central and won't apply cleanly when I land bug 1631452, but it should be trivial to unbitrot; just needs s/test30/test34/ or similar.)

Flags: needinfo?(emilio)

Also worth noting: this issue may happen for the testcase, or the reference case, or both.

Here's a log of a test-run where it only happened for the reference case:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318521583&repo=try&lineNumber=10336

Here are the data URIs for the mismatching snapshots from that log:
data URI for testcase snapshot, showing no margins

data URI for reference case, showing nonzero page margins, despite requesting all page-margins to be 0

I was not aware of anything like this, but this sounds like we infer the unwriteable margins from the printer and we sometimes either mess up or get different values from the printer.

Does something like the patch I attached fix it?

Flags: needinfo?(emilio)
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Whiteboard: [print2020_v83]
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2c4b604e95f6
Reset unwriteable margins in printpreview tests. r=jwatt
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch

Sadly, that doesn't help.

Here's a try run showing that we still hit this issue with the now-in-tree pages-per-sheet tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee577b46f852d64e52a9fcd442627c9479902857

(To reproduce this issue with those tests, just do a try run with https://hg.mozilla.org/mozilla-central/rev/dbaa8eaf2753 reverted -- that's the commit that added huge amounts of Windows-specific fuzz to work around this issue. This^ try run that I linked in this comment is simply testing the commit that came just before dbaa8eaf2753 (which did include the patch that emilio landed on this bug here).

Blocks: 1680838
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: