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
•