Column-span is not working if the multi-column container is <body> element
Categories
(Core :: Layout: Columns, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox66 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
See attached test case.
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 1•5 years ago
|
||
To reproduce, execute "GECKO_FRAMECTOR_DEBUG_FLAGS=really-noisy-content-updates ./mach run". List()'s second argument should be a const char*, not an integer. We can fix the bug by omitting it because the default value of the argument is an empty string.
|
|
Assignee | |
Comment 2•5 years ago
|
||
The elements under <body> are treat as content append, so their frames will be construct by nsCSSFrameConstructor::ContentAppended. This patch fixed only the simple "append" case which is appending to the last continuation of ::moz-column-content. For other more completx appending or inserting cases, we might need to reframe (bug 1504053). Depends on D16075
|
||
Comment 3•5 years ago
|
||
(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #1)
List()'s second argument should be a const char*, not an integer. We can
fix the bug by omitting it because the default value of the argument is
an empty string.
Where are you seeing the second arg being a const char*?
This is being called on aFirstNewContent->GetParent() which has type nsIContent*, and I'm seeing this in the nsIContent class definition:
virtual void List(FILE* out = stdout, int32_t aIndent = 0) const = 0;
https://searchfox.org/mozilla-central/source/dom/base/nsIContent.h#848-852
It's fine to omit the aIndent since it defaults to 0, but it also shouldn't make a functional difference...
|
|
||
Updated•5 years ago
|
|
|
||
Comment 4•5 years ago
|
||
Ah, I do see the nsIFrame list method takes a char* as its second arg. But your patch is touching calls on nsIContent as well on nsIFrame (the first call, at least). So, those changes need to be omitted from the patch.
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 5•5 years ago
|
||
Ah, I didn't aware some of the callers are nsIContent*. The buggy callers are nsContainerFrame::List() with definition:
void List(FILE* out = stderr, const char* aPrefix = "",
uint32_t aFlags = 0)
I should either rewrite the comment or fix only those buggy ones.
|
|
||
Comment 6•5 years ago
|
||
Yeah, it'd probably be fine to simplify the non-buggy nsIContent calls as well. But (per nits on phabricator) that should happen separately, to avoid grouping kinda-unrelated functional changes & non-functional changes together.
|
||
Updated•5 years ago
|
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/918042a8eb34 Part 1 - Adjust some callers of nsIFrame::List() to stop passing 0 as a const char* argument. r=dholbert https://hg.mozilla.org/integration/autoland/rev/3eafb595be4f Part 2 - Fix appending a subtree with column-span descendants under the ::moz-column-content. r=emilio
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/918042a8eb34
https://hg.mozilla.org/mozilla-central/rev/3eafb595be4f
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14906 for changes under testing/web-platform/tests
Description
•