Closed Bug 1562701 Opened 5 months ago Closed 5 months ago

Rework the reframing logic for `<details>` elements

Categories

(Core :: Layout, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently, when inserting a range of nodes into <details>, we treat <details> frames as if they have multiple insertion points. We then issue several single inserting commands, and insert the range of nodes one by one. However, what we do in inserting one node is merely reframing the <details> element. Also, the assertion that checks <details> should accept only single insertion cause bug 1547391 for multicol <details> with column-span:all children.

I think we can move the reframing logic for <details> into nsCSSFrameConstructor::WipeContainingBlock(), and do reframing if the range of nodes to be inserted contains <summary>; or do something smarter that checks the <summary> is going to be the main summary before do reframing.

Bug 1308080 would also make all this special logic unnecessary.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

Bug 1308080 would also make all this special logic unnecessary.

Right, bug 1308080 can make all this special logic go away. However, before we decide to take the patch, it still better to improve the special reframing logic to fix bug 1547391, and to reframe <details> only if a main <summary> is inserted.

Blocks: 1547391
Attachment #9075238 - Attachment description: Bug 1562701 Part 2 - Reframe <details> element only when inserting a main <summary>. → Bug 1562701 - Move the reframing logic of <details> element into WipeContainingBlock.

Re comment 0:

do reframing if the range of nodes to be inserted contains <summary>; or do something smarter that checks the <summary> is going to be the main summary before do reframing.

I realize it doesn't work if we only do reframing if the nodes to be inserted containing <summary>, because if we insert nodes without any <summary> at the place before <summary>, the summary frame won't be the first child ...

Anyway, I'll just move the logic to reframe <details> into WipeContainingBlock without introducing behavior changes.

Attachment #9075237 - Attachment is obsolete: true

David, review ping for this bug and bug 1547391. Thanks.

Flags: needinfo?(dbaron)
Flags: needinfo?(dbaron)
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/10faf756124c
Move the reframing logic of <details> element into WipeContainingBlock. r=dbaron
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17788 for changes under testing/web-platform/tests
Upstream PR merged
Regressions: 1579953
You need to log in before you can comment on or make changes to this bug.