commit 26b14fbd28750b1be47d72293c277fa1f936f69b
parent c207a896dff3989b8a10b1fa18df3eecfa3dea1a
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Tue, 6 Jan 2026 22:38:13 +0000
Bug 2008711 - When processing float insertions, compare placeholders rather than out of flow tree position. r=dholbert,layout-reviewers
It is true that the CompareTreePosition call goes through placeholders,
but it's not guaranteed to do so if the two frames are already children
of the CB.
Make the crashtest crash reliably without my patch by sprinkling some
getBoundingClientRect() around.
Differential Revision: https://phabricator.services.mozilla.com/D278044
Diffstat:
2 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/layout/base/crashtests/1489149.html b/layout/base/crashtests/1489149.html
@@ -1,52 +1,56 @@
<html class="reftest-wait">
-<head>
- <style>
- *|HTML {
- column-width: calc(15px)
- }
- *|HTML .class_1 {
- border-style: dotted;
- float: inline-end ! important
- }
+<style>
+ *|HTML {
+ column-width: calc(15px)
+ }
- * {
- block-size: calc(2px);
- }
- </style>
- <script noFuzz>
- function frameLoad() {
- setInterval(function() {
- document.documentElement.appendChild(o1)
- document.documentElement.appendChild(o2)
- }, 250)
- try { document.documentElement.appendChild(o4) } catch (e) {}
- try { xhr = new XMLHttpRequest() } catch (e) {}
- try { xhr.open('GET', 'data:text/html,1', false) } catch (e) {}
- try { xhr.send() } catch (e) {}
- try { document.documentElement.appendChild(o3) } catch (e) {}
- try { this.contentWindow.location.reload() } catch (e) {}
- }
+ *|HTML .class_1 {
+ border-style: dotted;
+ float: inline-end ! important
+ }
- function start() {
- o1 = document.createElement('del')
- o2 = document.createElement('track')
- o3 = document.createElement('video')
- o4 = document.createElement('video')
- o1.setAttribute('class', 'class_1')
- o2.setAttribute('class', 'class_1')
- o3.setAttribute('class', 'class_1')
- frame = document.createElement('iframe')
- frame.addEventListener('load', frameLoad)
- setTimeout(function(){
- document.documentElement.innerHTML="1";
- document.documentElement.removeAttribute("class");
- }, 1000);
- document.firstElementChild.appendChild(frame)
- }
+ * {
+ block-size: calc(2px);
+ }
+</style>
+<script>
+ function frameLoad() {
+ setInterval(function () {
+ document.documentElement.getBoundingClientRect();
+ document.documentElement.appendChild(o1)
+ document.documentElement.getBoundingClientRect();
+ document.documentElement.appendChild(o2)
+ }, 250)
+ document.documentElement.getBoundingClientRect();
+ try {document.documentElement.appendChild(o4)} catch (e) { }
+ try {xhr = new XMLHttpRequest()} catch (e) { }
+ try {xhr.open('GET', 'data:text/html,1', false)} catch (e) { }
+ try {xhr.send()} catch (e) { }
+ document.documentElement.getBoundingClientRect();
+ try {document.documentElement.appendChild(o3)} catch (e) { }
+ try {this.contentWindow.location.reload()} catch (e) { }
+ }
- document.addEventListener('DOMContentLoaded', start)
- </script>
- <body></body>
-</head>
+ function start() {
+ o1 = document.createElement('del')
+ o2 = document.createElement('track')
+ o3 = document.createElement('video')
+ o4 = document.createElement('video')
+ o1.setAttribute('class', 'class_1')
+ o2.setAttribute('class', 'class_1')
+ o3.setAttribute('class', 'class_1')
+ frame = document.createElement('iframe')
+ frame.addEventListener('load', frameLoad)
+ setTimeout(function () {
+ document.documentElement.innerHTML = "1";
+ document.documentElement.removeAttribute("class");
+ }, 1000);
+ document.documentElement.getBoundingClientRect();
+ document.firstElementChild.appendChild(frame)
+ }
+
+ document.addEventListener('DOMContentLoaded', start)
+</script>
+<body></body>
</html>
diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
@@ -1159,13 +1159,14 @@ MOZ_NEVER_INLINE void nsFrameConstructorState::ProcessFrameInsertions(
} else {
containingBlock->SetInitialChildList(aChildListID, std::move(aFrameList));
}
- } else if (aChildListID == FrameChildListID::Fixed ||
+ } else if (childList.IsEmpty() || aChildListID == FrameChildListID::Fixed ||
aChildListID == FrameChildListID::Absolute) {
// The order is not important for abs-pos/fixed-pos frame list, just
// append the frame items to the list directly.
mFrameConstructor->AppendFrames(containingBlock, aChildListID,
std::move(aFrameList));
} else {
+ MOZ_ASSERT(aChildListID == FrameChildListID::Float);
// Note that whether the frame construction context is doing an append or
// not is not helpful here, since it could be appending to some frame in
// the middle of the document, which means we're not necessarily
@@ -1174,34 +1175,31 @@ MOZ_NEVER_INLINE void nsFrameConstructorState::ProcessFrameInsertions(
// We need to make sure the 'append to the end of document' case is fast.
// So first test the last child of the containing block
nsIFrame* lastChild = childList.LastChild();
+ lastChild = lastChild->GetPlaceholderFrame();
// CompareTreePosition uses placeholder hierarchy for out of flow frames,
// so this will make out-of-flows respect the ordering of placeholders,
// which is great because it takes care of anonymous content.
nsIFrame* firstNewFrame = aFrameList.FirstChild();
+ firstNewFrame = firstNewFrame->GetPlaceholderFrame();
// Cache the ancestor chain so that we can reuse it if needed.
AutoTArray<const nsIFrame*, 20> firstNewFrameAncestors;
- const nsIFrame* notCommonAncestor = nullptr;
- if (lastChild) {
- notCommonAncestor = nsLayoutUtils::FillAncestors(
- firstNewFrame, containingBlock, &firstNewFrameAncestors);
- }
+ const nsIFrame* notCommonAncestor = nsLayoutUtils::FillAncestors(
+ firstNewFrame, containingBlock, &firstNewFrameAncestors);
- if (!lastChild || nsLayoutUtils::CompareTreePosition(
- lastChild, firstNewFrame, firstNewFrameAncestors,
- notCommonAncestor ? containingBlock : nullptr) < 0) {
- // no lastChild, or lastChild comes before the new children, so just
- // append
+ if (nsLayoutUtils::CompareTreePosition(
+ lastChild, firstNewFrame, firstNewFrameAncestors,
+ notCommonAncestor ? containingBlock : nullptr) < 0) {
+ // lastChild comes before the new children, so just append
mFrameConstructor->AppendFrames(containingBlock, aChildListID,
std::move(aFrameList));
} else {
// Try the other children. First collect them to an array so that a
// reasonable fast binary search can be used to find the insertion point.
AutoTArray<nsIFrame*, 128> children;
- for (nsIFrame* f = childList.FirstChild(); f != lastChild;
- f = f->GetNextSibling()) {
- children.AppendElement(f);
+ for (nsIFrame* f : childList) {
+ children.AppendElement(f->GetPlaceholderFrame());
}
nsIFrame* insertionPoint = nullptr;
