Check for sibling frames when propagating CSS page-name values
Categories
(Core :: CSS Parsing and Computation, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox107 | --- | fixed |
People
(Reporter: alaskanemily, Assigned: alaskanemily)
References
Details
Attachments
(1 file)
The exact situation is where we need to propagate the CSS page-name back up the frame tree from a frame that is the first child of a parent frame that already has a page-name set. The initial implementation of CSS page-name fragmentation in Bug 1740366 does not handle this case. This doesn't seem to be specifically needed for gsuite printing support, but should be fixed to properly implement the CSS page 3 spec.
We should probably implement this properly before named pages is preffed on by default.
This case is demonstrated by the test cases layout/reftests/css-page/page-name-siblings-002.html and layout/reftests/css-page/page-name-display-none-child.html
Assignee | ||
Updated•3 years ago
|
Comment 1•3 years ago
|
||
(In reply to Emily McDonough [:alaskanemily] from comment #0)
This case is demonstrated by the test cases layout/reftests/css-page/page-name-siblings-002.html and layout/reftests/css-page/page-name-display-none-child.html
Both of the testcases mentioned here are now marked as passing (though they were annotated as failing when this bug was filed).
Do we still have a correctness issue here? (If so, do we need a new testcase to demonstrate / test for that?)
(Noticed this because https://phabricator.services.mozilla.com/D152701 is moving a code-comment that refers to this bug.)
Assignee | ||
Comment 2•3 years ago
|
||
I believe there are still issues here. Since we link back to this bug in that code, I'll note what my concerns are:
In that code review at layout/base/nsCSSFrameConstructor.cpp
line 9661, we are checking if somewhere in the frame's direct ancestors that we have the mStartPageValue
value set on the nsIFrame::PageValuesProperty()
. However, because this code will run before the child frames of this frame (f
in this code) are created, if f
has a first child with a different page-name, then when this code runs for that child it will see the mStartPageValue
we set for f
and not set that value.
This only affects mStartPageValue
in this implementation because we unconditionally set mEndPageValue
. To fix this, we should replace the null-check on mStartPageValue
with a check that the previous frame we inspected (a child of frame
in that loop) is frame
's first child.
We don't have enough tests for complex/vertical frame trees that would catch this yet, and instead only the simplest case is working (which is those tests that are now working were for).
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
After testing, I believe that this was likely fixed by moving page value propagation in Bug 1779645 (or was possibly never actually an issue). I'm repurposing this bug for the same code change that was planned, as it will be needed for lazy page-value setting on frames.
Updated•3 years ago
|
Comment 6•3 years ago
|
||
bugherder |
Description
•