Bug 1304441 Part 4 - Fix rendering error if summary has ib-split.
The old logic moves only the primary frame of summary element to the
front of the child list. When the summary has ib-split, we'll need to
move all the summary frames to the front.
However, when details has ::first-line, things gets complicated. The
logic to move the summary frames to the front is still incorrect because
part of the summary frames will be wrapped in nsFirstLineFrame. This is
not easy to fix, so I leave it for now since I don't feel ib-split
summary with details::first-line is a common use case.
Change MOZ_ASSERT to NS_ASSERTION in CheckValidMainSummary() since it's
not fatal and it also allows us to use asserts in reftest.list for the
unfixed issue.
open-details-first-line-1.html asserts because the first child in
nsFirstLineFrame is "\n".
MozReview-Commit-ID: ERhH4vLh8XD
--- a/layout/generic/DetailsFrame.cpp
+++ b/layout/generic/DetailsFrame.cpp
@@ -53,20 +53,36 @@ DetailsFrame::SetInitialChildList(ChildL
if (isOpen) {
// If details is open, the first summary needs to be rendered as if it is
// the first child.
for (nsIFrame* child : aChildList) {
HTMLSummaryElement* summary =
HTMLSummaryElement::FromContent(child->GetContent());
if (summary && summary->IsMainSummary()) {
- // Take out the first summary frame and insert it to the beginning of
- // the list.
- aChildList.RemoveFrame(child);
- aChildList.InsertFrame(nullptr, nullptr, child);
+ // XXX: If content of |child| is the main summary, but is not the
+ // first continuation or ib-split sibling, some of the previous
+ // sibling might be wrapped in a wrapper frame like
+ // nsFirstLetterFrame. Because we do not deal perfectly in this case,
+ // we move all the main summary frames only if they're all direct
+ // children of the details frame.
+ if (nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(child)) {
+ // Take out the frames generated by first summary element and insert
+ // them to the beginning of the list.
+ nsIFrame* lastSummaryFrame =
+ nsLayoutUtils::LastContinuationOrIBSplitSibling(child);
+ nsFrameList framesAfterSummary = aChildList.RemoveFramesAfter(lastSummaryFrame);
+ nsFrameList summaryFrames = aChildList.RemoveFramesAfter(child->GetPrevSibling());
+
+ MOZ_ASSERT(!summaryFrames.IsEmpty(), "We must have at least a primary frame!");
+ aChildList.InsertFrames(nullptr, nullptr, summaryFrames);
+ if (!framesAfterSummary.IsEmpty()) {
+ aChildList.AppendFrames(nullptr, framesAfterSummary);
+ }
+ }
break;
}
}
}
#ifdef DEBUG
CheckValidMainSummary(aChildList);
#endif
@@ -90,19 +106,19 @@ DetailsFrame::CheckValidMainSummary(cons
} else if (child->GetType() == nsGkAtoms::lineFrame) {
// The content of the nsFirstLineFrame is the details element, so we can
// do recursive call here.
if (CheckValidMainSummary(child->PrincipalChildList())) {
return true;
}
}
} else {
- MOZ_ASSERT(!summary || !summary->IsMainSummary(),
- "Rest of the children are neither summary elements nor"
- " the main summary!");
+ NS_ASSERTION(!summary || !summary->IsMainSummary(),
+ "Rest of the children are neither summary elements nor"
+ " the main summary!");
}
}
return false;
}
#endif
void
DetailsFrame::DestroyFrom(nsIFrame* aDestructRoot)
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/open-details-first-line-1.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+ <style>
+ summary {
+ display: inline; /* ::first-line appiles only to inline element. */
+ }
+
+ details::first-line {
+ color: blue;
+ }
+ </style>
+ <body>
+ <details open>
+ <summary>Summary
+ <!-- Need ib-split so that the summary has multiple frames. -->
+ <div>Block in summary</div>
+ </summary>
+ <span>This is the details.</span>
+ </details>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/open-details-first-line-2.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+ <style>
+ summary {
+ display: inline; /* ::first-line appiles only to inline element. */
+ }
+
+ details::first-line {
+ color: blue;
+ }
+ </style>
+ <body>
+ <details open>
+ <span>This is the details.</span>
+ <summary>Summary
+ <!-- Need ib-split so that the summary has multiple frames. -->
+ <div>Block in summary</div>
+ </summary>
+ </details>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/open-details-first-line-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+ <style>
+ div#details::first-line {
+ color: blue;
+ }
+ </style>
+ <body>
+ <div id="details">
+ <span>Summary
+ <div>Block in summary</div>
+ </span>
+ <span>This is the details.</span>
+ </div>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/open-summary-inline-style-ref.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+ <body>
+ <div>
+ <span>Summary
+ <div>Block in summary</div>
+ </span>
+ <p>This is the details.</p>
+ </div>
+ </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/open-summary-inline-style.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+ - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html>
+ <style>
+ summary {
+ display: inline;
+ }
+ </style>
+ <body>
+ <details open>
+ <p>This is the details.</p>
+ <!-- Make summary the second element child so that layout will try to
+ render it as the first child. -->
+ <summary>Summary
+ <!-- Need ib-split so that the summary has multiple frames. -->
+ <div>Block in summary</div>
+ </summary>
+ </details>
+ </body>
+</html>
--- a/layout/reftests/details-summary/reftest.list
+++ b/layout/reftests/details-summary/reftest.list
@@ -6,16 +6,17 @@ pref(dom.details_element.enabled,false)
pref(dom.details_element.enabled,false) == no-summary.html disabled-no-summary-ref.html
# Basic <summary> handling
== multiple-summary.html single-summary.html
== open-multiple-summary.html open-multiple-summary-ref.html
== summary-not-first-child.html single-summary.html
== open-summary-not-first-child.html open-single-summary.html
== open-summary-block-style.html open-summary-block-style-ref.html
+== open-summary-inline-style.html open-summary-inline-style-ref.html
== no-summary.html no-summary-ref.html
== open-no-summary.html open-no-summary-ref.html
== summary-not-in-details.html summary-not-in-details-ref.html
== summary-not-direct-child.html summary-not-direct-child-ref.html
== float-in-summary.html float-in-summary-ref.html
# Add elements dynamically
== dynamic-add-single-summary.html open-single-summary.html
@@ -66,16 +67,18 @@ pref(dom.details_element.enabled,false)
== details-display-inline.html details-display-inline-ref.html
== details-percentage-height-children.html details-percentage-height-children-ref.html
== details-absolute-children.html details-absolute-children-ref.html
== details-three-columns.html details-three-columns-ref.html
== details-writing-mode.html details-writing-mode-ref.html
== details-in-ol.html details-in-ol-ref.html
== summary-three-columns.html summary-three-columns-ref.html
== details-first-line.html details-first-line-ref.html
+asserts(3) == open-details-first-line-1.html open-details-first-line-ref.html
+fails asserts(3) == open-details-first-line-2.html open-details-first-line-ref.html
# Dispatch mouse click to summary
== mouse-click-single-summary.html open-single-summary.html
== mouse-click-twice-single-summary.html single-summary.html
== mouse-click-open-single-summary.html single-summary.html
== mouse-click-twice-open-single-summary.html open-single-summary.html
== mouse-click-open-second-summary.html open-multiple-summary.html
== mouse-click-overflow-hidden-details.html overflow-hidden-open-details.html
