corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1304441 Part 3 - Insert main summary's frame construction item at front of the list. r=bz

Browse Source 
Change the logic that moves the main summary to the front from operating
on generated frames in DetailsFrame::SetInitialChildList() to operating
on frame construction item list in AddFrameConstructionItemsInternal()
so that it will be correct when cooperating with ::first-line.

The root cause of the bug reported is because when specifying
::first-line on details element, the first frame of summary element,
which is generated due to ib-split, will be wrapped in nsFirstLineFrame.
The original code fails to find the summary frame in the wrapper frame
and triggers assertion because of the second ib-split summary frame. To
fix that, we need to descend into the child list of wrapper frames when
checking the main summary.

Add original test case as a crashtest as well as reftests to clearly
reproduce the issue.

Note that in the reftest, the blue color in ::first-line is applied
incorrectly to the second line in the summary due to bug 520605.

MozReview-Commit-ID: Bv4Vcvxp6pY
This commit is contained in:
Ting-Yu Lin
2016-10-05 14:43:32 +08:00
parent a4f928106c
commit cc5f5f88a0
16 changed files with 247 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
layout/base/nsCSSFrameConstructor.cpp
View File

@@ -5896,10 +5896,23 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
return;
}
FrameConstructionItem* item =
aItems.AppendItem(data, aContent, aTag, aNameSpaceID,
pendingBinding, styleContext.forget(),
aSuppressWhiteSpaceOptimizations, aAnonChildren);
FrameConstructionItem* item = nullptr;
if (details && details->IsDetailsEnabled() && details->Open()) {
auto* summary = HTMLSummaryElement::FromContentOrNull(aContent);
if (summary && summary->IsMainSummary()) {
// If details is open, the main summary needs to be rendered as if it is
// the first child, so add the item to the front of the item list.
item = aItems.PrependItem(data, aContent, aTag, aNameSpaceID,
pendingBinding, styleContext.forget(),
aSuppressWhiteSpaceOptimizations, aAnonChildren);
}
}
if (!item) {
item = aItems.AppendItem(data, aContent, aTag, aNameSpaceID,
pendingBinding, styleContext.forget(),
aSuppressWhiteSpaceOptimizations, aAnonChildren);
}
item->mIsText = isText;
item->mIsGeneratedContent = isGeneratedContent;
item->mIsAnonymousContentCreatorContent =

21
layout/base/nsCSSFrameConstructor.h
View File

@@ -862,6 +862,27 @@ private:
return item;
}
// Arguments are the same as AppendItem().
FrameConstructionItem* PrependItem(const FrameConstructionData* aFCData,
nsIContent* aContent,
nsIAtom* aTag,
int32_t aNameSpaceID,
PendingBinding* aPendingBinding,
already_AddRefed<nsStyleContext>&& aStyleContext,
bool aSuppressWhiteSpaceOptimizations,
nsTArray<nsIAnonymousContentCreator::ContentInfo>* aAnonChildren)
{
FrameConstructionItem* item =
new FrameConstructionItem(aFCData, aContent, aTag, aNameSpaceID,
aPendingBinding, aStyleContext,
aSuppressWhiteSpaceOptimizations,
aAnonChildren);
PR_INSERT_LINK(item, &mItems);
++mItemCount;
++mDesiredParentCounts[item->DesiredParentType()];
return item;
}
void AppendUndisplayedItem(nsIContent* aContent,
nsStyleContext* aStyleContext) {
mUndisplayedItems.AppendElement(UndisplayedItem(aContent, aStyleContext));

37
layout/generic/DetailsFrame.cpp
View File

@@ -46,38 +46,17 @@ DetailsFrame::GetType() const
void
DetailsFrame::SetInitialChildList(ChildListID aListID, nsFrameList& aChildList)
{
if (aListID == kPrincipalList) {
HTMLDetailsElement* details = HTMLDetailsElement::FromContent(GetContent());
bool isOpen = details->Open();
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);
break;
}
}
}
#ifdef DEBUG
if (aListID == kPrincipalList) {
CheckValidMainSummary(aChildList);
#endif
}
#endif
nsBlockFrame::SetInitialChildList(aListID, aChildList);
}
#ifdef DEBUG
void
bool
DetailsFrame::CheckValidMainSummary(const nsFrameList& aFrameList) const
{
for (nsIFrame* child : aFrameList) {
@@ -86,7 +65,14 @@ DetailsFrame::CheckValidMainSummary(const nsFrameList& aFrameList) const
if (child == aFrameList.FirstChild()) {
if (summary && summary->IsMainSummary()) {
break;
return true;
} else if (child->GetContent() == GetContent()) {
// The child frame's content is the same as our content, which means
// it's a kind of wrapper frame. Descend into its child list to find
// main summary.
if (CheckValidMainSummary(child->PrincipalChildList())) {
return true;
}
}
} else {
NS_ASSERTION(!summary || !summary->IsMainSummary(),
@@ -94,6 +80,7 @@ DetailsFrame::CheckValidMainSummary(const nsFrameList& aFrameList) const
"or are not the main summary!");
}
}
return false;
}
#endif

4
layout/generic/DetailsFrame.h
View File

@@ -40,8 +40,8 @@ public:
#ifdef DEBUG
// Check the frame of the main summary element is the first child in the frame
// list.
void CheckValidMainSummary(const nsFrameList& aFrameList) const;
// list. Returns true if we found the main summary frame; false otherwise.
bool CheckValidMainSummary(const nsFrameList& aFrameList) const;
#endif
void SetInitialChildList(ChildListID aListID,

9
layout/generic/crashtests/1304441.html Normal file
View File

@@ -0,0 +1,9 @@
<details>
<summary>
<li>
<style>
summary{
all:initial
}
:first-child::first-line
{}

1
layout/generic/crashtests/crashtests.list
View File

@@ -639,3 +639,4 @@ load large-border-radius-dotted2.html
load 1297427-non-equal-centers.html
load 1278461-1.html
load 1278461-2.html
load 1304441.html

18
layout/reftests/details-summary/details-first-line-ref.html Normal file
View File

@@ -0,0 +1,18 @@
<!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>
</div>
</body>
</html>

24
layout/reftests/details-summary/details-first-line.html Normal file
View File

@@ -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>
<summary>Summary
<!-- Need ib-split so that the summary has multiple frames. -->
<div>Block in summary</div>
</summary>
<p>This is the details.</p>
</details>
</body>
</html>

24
layout/reftests/details-summary/open-details-first-line-1.html Normal file
View File

@@ -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>

24
layout/reftests/details-summary/open-details-first-line-2.html Normal file
View File

@@ -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>

19
layout/reftests/details-summary/open-details-first-line-ref.html Normal file
View File

@@ -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>

14
layout/reftests/details-summary/open-summary-inline-style-ref.html Normal file
View File

@@ -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>

22
layout/reftests/details-summary/open-summary-inline-style.html Normal file
View File

@@ -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>

14
layout/reftests/details-summary/open-summary-table-cell-style-ref.html Normal file
View File

@@ -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 style="display: table-cell;">Summary
<div>Block in summary</div>
</span>
<p>This is the details.</p>
</div>
</body>
</html>

21
layout/reftests/details-summary/open-summary-table-cell-style.html Normal file
View File

@@ -0,0 +1,21 @@
<!DOCTYPE html>
<!-- Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ -->
<html>
<style>
summary {
display: table-cell;
}
</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
<div>Block in summary</div>
</summary>
</details>
</body>
</html>

5
layout/reftests/details-summary/reftest.list
View File

@@ -11,6 +11,8 @@ pref(dom.details_element.enabled,false) == no-summary.html disabled-no-summary-r
== 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
== open-summary-table-cell-style.html open-summary-table-cell-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
@@ -70,6 +72,9 @@ pref(dom.details_element.enabled,false) == no-summary.html disabled-no-summary-r
== 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
== open-details-first-line-1.html open-details-first-line-ref.html
== 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