Bug 1258657 Part 1 - Change summary default style to "display: list-item" to match html spec. r=bz
authorTing-Yu Lin <tlin@mozilla.com>
Wed, 13 Apr 2016 13:34:14 +0800
changeset 330870 c9894253461b30b2d6b7ca05d4c44b148e51c6ef
parent 330869 8c370d8407563729afc60c7695529e32e5873905
child 330871 1c23caab8a0d0cc212b9675598afbffbbc2d0588
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1258657
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1258657 Part 1 - Change summary default style to "display: list-item" to match html spec. r=bz Per html spec, the disclosure triangle can be generated via "display: list-item", I removed the code to generate the triangle in SummaryFrame::SetInitialChildList(). That is, when a web page set "display: block" to the summary, the triangle will disappear, too. Now SummaryFrame does nothing and is going to be removed in Part 2. Also summary element should not increment the counter as hinted as "counter-increment: list-item 0" in the spec. Hence the change in nsBlockFrame::RenumberListsFor(). The rendering hint in html spec: https://html.spec.whatwg.org/multipage/rendering.html#the-details-and-summary-elements MozReview-Commit-ID: DELGYFe3zGX
layout/generic/SummaryFrame.cpp
layout/generic/nsBlockFrame.cpp
layout/reftests/details-summary/details-in-ol-ref.html
layout/reftests/details-summary/details-in-ol.html
layout/reftests/details-summary/open-summary-block-style-ref.html
layout/reftests/details-summary/open-summary-block-style.html
layout/reftests/details-summary/reftest.list
layout/style/res/html.css
--- a/layout/generic/SummaryFrame.cpp
+++ b/layout/generic/SummaryFrame.cpp
@@ -30,21 +30,9 @@ SummaryFrame::GetType() const
 {
   return nsGkAtoms::summaryFrame;
 }
 
 void
 SummaryFrame::SetInitialChildList(ChildListID aListID, nsFrameList& aChildList)
 {
   nsBlockFrame::SetInitialChildList(aListID, aChildList);
-
-  // Construct the disclosure triangle if it's the main summary. We leverage the
-  // list-item property and nsBulletFrame to draw the triangle. Need to set
-  // list-style-type for :moz-list-bullet in html.css.
-  // TODO: Bug 1221416 for styling the disclosure triangle.
-  if (aListID == kPrincipalList) {
-    auto* summary = HTMLSummaryElement::FromContent(GetContent());
-    if (summary->IsMainSummary() &&
-        StyleDisplay()->mDisplay != NS_STYLE_DISPLAY_LIST_ITEM) {
-      CreateBulletFrameForListItem(true, true);
-    }
-  }
 }
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -47,16 +47,18 @@
 #include "nsDisplayList.h"
 #include "nsCSSAnonBoxes.h"
 #include "nsCSSFrameConstructor.h"
 #include "nsRenderingContext.h"
 #include "TextOverflow.h"
 #include "nsIFrameInlines.h"
 #include "CounterStyleManager.h"
 #include "nsISelection.h"
+#include "mozilla/dom/HTMLDetailsElement.h"
+#include "mozilla/dom/HTMLSummaryElement.h"
 #include "mozilla/StyleSetHandle.h"
 #include "mozilla/StyleSetHandleInlines.h"
 
 #include "nsBidiPresUtils.h"
 
 static const int MIN_LINES_NEEDING_CURSOR = 20;
 
 static const char16_t kDiscCharacter = 0x2022;
@@ -6913,17 +6915,16 @@ nsBlockFrame::CreateBulletFrameForListIt
 
   // Create bullet frame
   nsBulletFrame* bullet = new (shell) nsBulletFrame(kidSC);
   bullet->Init(mContent, this, nullptr);
 
   // If the list bullet frame should be positioned inside then add
   // it to the flow now.
   if (aListStylePositionInside) {
-
     nsFrameList bulletList(bullet, bullet);
     AddFrames(bulletList, nullptr);
     Properties().Set(InsideBulletProperty(), bullet);
     AddStateBits(NS_BLOCK_FRAME_HAS_INSIDE_BULLET);
   } else {
     nsFrameList* bulletList = new (shell) nsFrameList(bullet, bullet);
     Properties().Set(OutsideBulletProperty(), bulletList);
     AddStateBits(NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET);
@@ -7085,16 +7086,25 @@ nsBlockFrame::RenumberListsFor(nsPresCon
 
   // drill down through any wrappers to the real frame
   kid = kid->GetContentInsertionFrame();
 
   // possible there is no content insertion frame
   if (!kid)
     return false;
 
+  // Do not renumber list for summary elements.
+  if (HTMLDetailsElement::IsDetailsEnabled()) {
+    HTMLSummaryElement* summary =
+      HTMLSummaryElement::FromContent(kid->GetContent());
+    if (summary && summary->IsMainSummary()) {
+      return false;
+    }
+  }
+
   bool kidRenumberedABullet = false;
 
   // If the frame is a list-item and the frame implements our
   // block frame API then get its bullet and set the list item
   // ordinal.
   if (NS_STYLE_DISPLAY_LIST_ITEM == display->mDisplay) {
     // Make certain that the frame is a block frame in case
     // something foreign has crept in.
--- a/layout/reftests/details-summary/details-in-ol-ref.html
+++ b/layout/reftests/details-summary/details-in-ol-ref.html
@@ -2,16 +2,29 @@
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 
 <html>
   <body>
     <ol>
       <li>First item
         <div>
-          <summary>Summary</summary>
+          <summary>Summary
+            <ul>
+              <li>First unordered item in summary</li>
+              <li>Second unordered item in summary</li>
+            </ul>
+            <div>
+              <ol>
+                <li>First item in summary</li>
+                <li>Second item in summary</li>
+              </ol>
+            </div>
+          </summary>
           <p>This is the details.</p>
+          <li>First item in details</li>
+          <li>Second item in details</li>
         </div>
       </li>
       <li>Second item</li>
     </ol>
   </body>
 </html>
--- a/layout/reftests/details-summary/details-in-ol.html
+++ b/layout/reftests/details-summary/details-in-ol.html
@@ -9,16 +9,32 @@
     list-style-type: none;
   }
   </style>
   <body>
     <!-- Test <ol> numbering is not affected by <details>. -->
     <ol>
       <li>First item
         <details open>
-          <summary>Summary</summary>
+          <summary>Summary
+            <ul>
+              <li>First unordered item in summary</li>
+              <li>Second unordered item in summary</li>
+            </ul>
+            <div>
+              <ol>
+                <li>First item in summary</li>
+                <li>Second item in summary</li>
+              </ol>
+            </div>
+          </summary>
           <p>This is the details.</p>
+          <!-- Although html spec does not allow <li> inside <details>, we
+               deliberately omit the <ol> to test the renumbering isn't affected
+               by the summary. -->
+          <li>First item in details</li>
+          <li>Second item in details</li>
         </details>
       </li>
       <li>Second item</li>
     </ol>
   </body>
 </html>
copy from layout/reftests/details-summary/open-summary-block-style.html
copy to layout/reftests/details-summary/open-summary-block-style-ref.html
--- a/layout/reftests/details-summary/open-summary-block-style.html
+++ b/layout/reftests/details-summary/open-summary-block-style-ref.html
@@ -1,12 +1,12 @@
 <!DOCTYPE html>
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 
 <html>
   <body>
-    <details open>
-      <summary style="display: list-item; list-style-type: disc; list-style-position: inside;">Summary</summary>
+    <div>
+      <div>Summary</div>
       <p>This is the details.</p>
-    </details>
+    </div>
   </body>
 </html>
--- a/layout/reftests/details-summary/open-summary-block-style.html
+++ b/layout/reftests/details-summary/open-summary-block-style.html
@@ -1,12 +1,13 @@
 <!DOCTYPE html>
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 
 <html>
   <body>
     <details open>
-      <summary style="display: list-item; list-style-type: disc; list-style-position: inside;">Summary</summary>
+      <!-- Test the disclosure triangle is gone. -->
+      <summary style="display: block;">Summary</summary>
       <p>This is the details.</p>
     </details>
   </body>
 </html>
--- a/layout/reftests/details-summary/reftest.list
+++ b/layout/reftests/details-summary/reftest.list
@@ -3,17 +3,17 @@ pref(dom.details_element.enabled,false) 
 pref(dom.details_element.enabled,false) == open-single-summary.html disabled-single-summary-ref.html
 pref(dom.details_element.enabled,false) == no-summary.html disabled-no-summary-ref.html
 
 # Basic <summary> handling
 pref(dom.details_element.enabled,true) == multiple-summary.html single-summary.html
 pref(dom.details_element.enabled,true) == open-multiple-summary.html open-multiple-summary-ref.html
 pref(dom.details_element.enabled,true) == summary-not-first-child.html single-summary.html
 pref(dom.details_element.enabled,true) == open-summary-not-first-child.html open-single-summary.html
-pref(dom.details_element.enabled,true) == open-summary-block-style.html open-single-summary.html
+pref(dom.details_element.enabled,true) == open-summary-block-style.html open-summary-block-style-ref.html
 pref(dom.details_element.enabled,true) == no-summary.html no-summary-ref.html
 pref(dom.details_element.enabled,true) == open-no-summary.html open-no-summary-ref.html
 pref(dom.details_element.enabled,true) == summary-not-in-details.html summary-not-in-details-ref.html
 pref(dom.details_element.enabled,true) == summary-not-direct-child.html summary-not-direct-child-ref.html
 pref(dom.details_element.enabled,true) == float-in-summary.html float-in-summary-ref.html
 
 # Add elements dynamically
 pref(dom.details_element.enabled,true) == dynamic-add-single-summary.html open-single-summary.html
--- a/layout/style/res/html.css
+++ b/layout/style/res/html.css
@@ -768,23 +768,31 @@ audio:not([controls]) {
 video > .caption-box {
   position: relative;
   overflow: hidden;
 }
 
 /* details & summary */
 /* Need to revert Bug 1259889 Part 2 when removing details preference. */
 @supports -moz-bool-pref("dom.details_element.enabled") {
-  details > summary::-moz-list-bullet {
-    list-style-type: disclosure-closed;
+  details > summary:first-of-type,
+  details > summary:-moz-native-anonymous {
+    display: list-item;
+    list-style: disclosure-closed inside;
   }
 
-  details[open] > summary::-moz-list-bullet {
+  details[open] > summary:first-of-type,
+  details[open] > summary:-moz-native-anonymous {
     list-style-type: disclosure-open;
   }
+
+  details > summary:first-of-type > * {
+    /* Cancel "list-style-position: inside" inherited from summary. */
+    list-style-position: initial;
+  }
 }
 
 /* emulation of non-standard HTML <marquee> tag */
 marquee {
   inline-size: -moz-available;
   display: inline-block;
   vertical-align: text-bottom;
   text-align: start;