Height computation for (nested?) box is somewhat different in printing
Categories
(Core :: Layout: Tables, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox78 | --- | fixed |
People
(Reporter: hiro, Assigned: TYLin)
References
Details
(Whiteboard: [layout:backlog:78])
Attachments
(7 files)
compasison.png
Open below file on LayoutDebugger on "PagedMode" and scroll down to the end of the contents and see the blue box height on the last page. You can see the difference.
https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/reftests/pagination/820496-1.html
https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/reftests/pagination/820496-1-ref.html
I am not sure this is a bug which has been already filed or not (I couldn't find any of relevant bugs though). This is one of reasons that some of reftests in layout/reftest/pagination fail with jgraham's wpt printing machinery.
|
Reporter | |
Comment 1•5 years ago
•
|
||
820496-1.html might not be a good example to represent this issue, since the blue background color in the test html is on <body> element, so I started thinking the blue background should be rendered on the whole area of the last page, whereas the blue background color in the ref html is on a <div>.
https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/reftests/bugs/422678-1.html might be a better example to show this issue. I am attaching two screenshots to compare with the test and the ref images.
|
|
Reporter | |
Comment 2•5 years ago
|
||
|
|
Reporter | |
Comment 3•5 years ago
|
||
I noticed that there is 4px difference between the vis-overflow of the second PageContent in test html and reference html. vis-overflow=(0,0,23040,37980) in the reference and vis-overflow=(0,0,23040,37740) in the test. I have no idea where this 4px difference comes from, maybe we accidentally factor a margin/padding for an nsIFrame when we do paginate?
|
|
Reporter | |
Updated•5 years ago
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 4•5 years ago
•
|
||
This is a table layout bug. The table cell frame doesn't handle border and padding correctly in a paginated context.
For example, it didn't apply skip sides (by using ApplySkipSides) for the borderPadding. Also, it unconditionally subtracts block-axis border/padding from available block-size for its children here, and add it back after the cell size is known. This can go wrong even if the skip sides are applied. For example, if the table cell needs a second page, its bottom border/padding needs to be skipped on the first page.
Hiro, did you see other reftest failures other than table layout under jgraham's wpt printing machinery? If yes, I think it's worth filing such bugs.
(NI myself to revisit this later.)
|
|
Reporter | |
Comment 5•5 years ago
|
||
(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #4)
Hiro, did you see other reftest failures other than table layout under jgraham's wpt printing machinery? If yes, I think it's worth filing such bugs.
I filed bug 1627571 for a reftest failure which is not including any table related elements.
Though I haven't audited all failure cases, it looks like most of them are related to table frames, e.g. table-page-break-before-auto-2.html is one of them.
|
||
Updated•5 years ago
|
|
|
Reporter | |
Comment 6•5 years ago
|
||
FYI, I saw "Set table cell incomplete 0x7fc1f2f27c10" when I run layout/reftests/pagination/1406291-1.html with the wpt printing stuff. It may or may not be related to this bug.
|
|
Assignee | |
Comment 7•5 years ago
|
||
A test case adapted from 422678-1.html. The table cell's border and padding look pretty wrong in paged mode.
|
|
Assignee | |
Comment 8•5 years ago
|
||
The <td>'s bottom border/padding should be skipped in the first page, and its top border/padding should be skipped in the second page as well.
|
|
Assignee | |
Comment 9•5 years ago
|
||
Before this patch, nsTableCellFrame never skips border and padding. If
it needs to be broken into multiple parts, the cell's border and padding
didn't render correctly, and its children didn't have the correct
available space to reflow. (A side effect of this is that it renders
correctly if it has box-decoration-break:clone.)
table-nested-1308876-1-ref.html is modified to reflect the rendering
after applying this patch, which isn't necessarily correct (based on the
file's comment).
|
||
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 10•5 years ago
|
||
-
2a and 2b are copied from 1a and 1b with "box-decoration-break: clone"
added. -
3a and 3b are copied from 1a and 1b. They test that the content fit
but the table cell's bottom border and padding cannot fit. As of this
bug, their rendering matches the block frame's, but 3a can now trigger a
"data loss" warning and 3b an assertion in
nsTableRowGroupFrame::SplitRowGroup. That might need a deeper look
into table's pagination to have a proper fix.
Depends on D71096
|
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
||
Comment 11•5 years ago
|
||
Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)
|
||
Comment 12•5 years ago
|
||
|
||
Comment 13•5 years ago
|
||
Backed out for reftest failure on table-nested-1308876-1.xhtml
Backout link: https://hg.mozilla.org/integration/autoland/rev/a634a062fb973d758283e9b1229ce26765265c7d
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302558309&repo=autoland&lineNumber=41721
|
|
Assignee | |
Comment 14•5 years ago
|
||
By looking at the log, table-nested-1308876-1.xhtml has a fuzzy pixel on macOS.
A try run with fuzzy annotation added https://treeherder.mozilla.org/#/jobs?repo=try&collapsedPushes=652615&revision=3e684edeb77c93c4021a3aa694170e9298b7fa30
|
|
||
Comment 15•5 years ago
|
||
|
|
||
Updated•5 years ago
|
|
||
Comment 16•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/1cdfd91b0a4f
https://hg.mozilla.org/mozilla-central/rev/751209e30850
Description
•