From 84d3b44b869ac3ef1f36126ebe9dde56267ff783 Mon Sep 17 00:00:00 2001
From: "Matt A. Tobin" <email@mattatobin.com>
Date: Fri, 17 Apr 2020 06:09:37 -0400
Subject: [PATCH] Bug 1377648 - Fix HTMLSummaryElement::IsSummary() on removing
 the element

* Use inFlowFrame to check the target frame is summary and its parent is details
* Check summary frame instead of summary element on removing the summary

Tag #1375
---
 layout/base/nsCSSFrameConstructor.cpp        | 38 +++++++++++---------
 layout/generic/DetailsFrame.cpp              |  9 +++++
 layout/generic/DetailsFrame.h                |  6 ++++
 layout/reftests/details-summary/reftest.list |  4 +++
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
index 57410a55a5..8b89b2a223 100644
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -9395,23 +9395,6 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame,
     return true;
   }
 
-  if (insertionFrame &&
-      aFrame->GetParent()->GetType() == nsGkAtoms::detailsFrame) {
-    HTMLSummaryElement* summary =
-      HTMLSummaryElement::FromContent(insertionFrame->GetContent());
-
-    if (summary && summary->IsMainSummary()) {
-      // When removing a summary, we should reframe the parent details frame to
-      // ensure that another summary is used or the default summary is
-      // generated.
-      RecreateFramesForContent(aFrame->GetParent()->GetContent(),
-                               false, REMOVE_FOR_RECONSTRUCTION,
-                               aDestroyedFramesFor);
-      return true;
-    }
-  }
-
-  // Now check for possibly needing to reconstruct due to a pseudo parent
   nsIFrame* inFlowFrame =
     (aFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) ?
       aFrame->GetPlaceholderFrame() : aFrame;
@@ -9419,6 +9402,27 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame,
   MOZ_ASSERT(inFlowFrame == inFlowFrame->FirstContinuation(),
              "placeholder for primary frame has previous continuations?");
   nsIFrame* parent = inFlowFrame->GetParent();
+
+  if (parent && parent->GetType() == nsGkAtoms::detailsFrame) {
+    HTMLSummaryElement* summary =
+      HTMLSummaryElement::FromContent(aFrame->GetContent());
+    DetailsFrame* detailsFrame = static_cast<DetailsFrame*>(parent);
+
+    // Unlike adding summary element cases, we need to check children of the
+    // parent details frame since at this moment the summary element has been
+    // already removed from the parent details element's child list.
+    if (summary && detailsFrame->HasMainSummaryFrame(aFrame)) {
+      // When removing a summary, we should reframe the parent details frame to
+      // ensure that another summary is used or the default summary is
+      // generated.
+      RecreateFramesForContent(parent->GetContent(),
+                               false, REMOVE_FOR_RECONSTRUCTION,
+                               aDestroyedFramesFor);
+      return true;
+    }
+  }
+
+  // Now check for possibly needing to reconstruct due to a pseudo parent
   // For the case of ruby pseudo parent, effectively, only pseudo rb/rt frame
   // need to be checked here, since all other types of parent will be catched
   // by "Check ruby containers" section below.
diff --git a/layout/generic/DetailsFrame.cpp b/layout/generic/DetailsFrame.cpp
index 1d28bbe960..389a476cba 100644
--- a/layout/generic/DetailsFrame.cpp
+++ b/layout/generic/DetailsFrame.cpp
@@ -129,3 +129,12 @@ DetailsFrame::AppendAnonymousContentTo(nsTArray<nsIContent*>& aElements,
     aElements.AppendElement(mDefaultSummary);
   }
 }
+
+bool
+DetailsFrame::HasMainSummaryFrame(nsIFrame* aSummaryFrame)
+{
+  nsIFrame* firstChild =
+    nsPlaceholderFrame::GetRealFrameFor(mFrames.FirstChild());
+
+  return aSummaryFrame == firstChild;
+}
diff --git a/layout/generic/DetailsFrame.h b/layout/generic/DetailsFrame.h
index 1eb4ea87be..4ae177f16f 100644
--- a/layout/generic/DetailsFrame.h
+++ b/layout/generic/DetailsFrame.h
@@ -54,6 +54,12 @@ public:
 
   void AppendAnonymousContentTo(nsTArray<nsIContent*>& aElements,
                                 uint32_t aFilter) override;
+  // Returns true if |aSummaryFrame| is the main summary (i.e. the first child
+  // of this details frame).
+  // This function is used when the summary element is removed from the parent
+  // details element since at that moment the summary element has been already
+  // removed from the details element children.
+  bool HasMainSummaryFrame(nsIFrame* aSummaryFrame);
 
 private:
   nsCOMPtr<nsIContent> mDefaultSummary;
diff --git a/layout/reftests/details-summary/reftest.list b/layout/reftests/details-summary/reftest.list
index e96581ad45..a972cf4980 100644
--- a/layout/reftests/details-summary/reftest.list
+++ b/layout/reftests/details-summary/reftest.list
@@ -101,3 +101,7 @@ fuzzy(1,1) == mouse-click-twice-float-details.html float-details.html # Bug 1316
 == details-before.html single-summary.html
 == open-details-after.html open-single-summary.html
 == open-details-before.html open-single-summary.html
+
+# Move summary element
+== move-float-summary-to-different-details.html move-float-summary-to-different-details-ref.html
+== move-position-absolute-summary-to-different-details.html move-position-absolute-summary-to-different-details-ref.html