Closed Bug 1503420 Opened 7 years ago Closed 7 years ago

Column-span is not working if the multi-column container is <body> element

Categories

(Core :: Layout: Columns, defect, P3)

Product:
Core
Component:
Layout: Columns
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox66 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(3 files)

body-column.html
370 bytes, text/html
Details
Bug 1503420 Part 1 - Adjust some callers of nsIFrame::List() to stop passing 0 as a const char* argument.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1503420 Part 2 - Fix appending a subtree with column-span descendants under the ::moz-column-content.
47 bytes, text/x-phabricator-request
Details | Review
Attached file body-column.html —
See attached test case.
Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
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.
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

(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...

Flags: needinfo?(aethanyc)

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.

Flags: needinfo?(aethanyc)

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)

https://searchfox.org/mozilla-central/rev/5053031ba7621fa8f63f42de4c204ab3561e4e59/layout/generic/nsContainerFrame.h#73

I should either rewrite the comment or fix only those buggy ones.

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.

Attachment #9035397 - Attachment description: Bug 1503420 Part 1 - Fix crash when using GECKO_FRAMECTOR_DEBUG_FLAGS. → Bug 1503420 Part 1 - Adjust some callers of nsIFrame::List() to stop passing 0 as a const char* argument.
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

https://hg.mozilla.org/mozilla-central/rev/918042a8eb34
https://hg.mozilla.org/mozilla-central/rev/3eafb595be4f

Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14906 for changes under testing/web-platform/tests
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: