Support 'disclosure-open' and 'disclosure-closed' for 'list-style-type'

property

* core/css, core/style, web_tests/external/wpt:
  Changes to recognize new keywords 'disclosure-open' and
  'disclosure-closed'.

* layout/layout_details_marker.*,
  layout/layout_tree_as_text.cc,
  paint/details_marker_painter.*:
  Changes to make DetailsMarkerPainter::GetCanonicalPath() static in
  order to share it with ListMarkerPainter.
  These files has no behavior changes.

* paint/list_marker_painter.cc:
  Calls DetailsMarkerPainter::GetCanonicalPath() to get a Path, and draw
  it on |context|.

* layout/list_marker.cc:
  Adjust the layout size for 'disclosure-*'.
  The size of 'circle', 'disc', 'square' is "ascent * 1 / 3", and it's
  too small for 'disclosure-*'.  We apply "font_height * 2 / 3" instead,
  and it's same as the default size of ::-webkit-details-marker.

* layout/list_marker_text.cc, wtf/text/character_names.h:
  Add text representations of 'disclosure-open' and 'disclosure-closed'.

This CL adds a simple rendering test. A following CL will add more
coverage.

I2S: https://t.co/1a7z357RV9

Bug: 590014
Change-Id: I1acad5a102c7cfcc91101fca283fde32fefae06f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2567548
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833566}

third_party/blink/renderer/core/css/css_properties.json5

@@ -2422,7 +2422,8 @@
       default_value: "ListStyleTypeData::CreateCounterStyle(\"disc\", nullptr)",
       type_name: "ListStyleTypeData",
       keywords: [
-        "disc", "circle", "square", "decimal", "decimal-leading-zero",
+        "disc", "circle", "square", "disclosure-open", "disclosure-closed",
+        "decimal", "decimal-leading-zero",
         "arabic-indic", "bengali", "cambodian", "khmer", "devanagari",
         "gujarati", "gurmukhi", "kannada", "lao", "malayalam", "mongolian",
         "myanmar", "oriya", "persian", "urdu", "telugu", "tibetan", "thai",

third_party/blink/renderer/core/css/css_value_id_mappings.h

@@ -185,6 +185,10 @@ inline EListStyleType CssValueIDToPlatformEnum(CSSValueID v) {
       return EListStyleType::kCircle;
     case CSSValueID::kSquare:
       return EListStyleType::kSquare;
+    case CSSValueID::kDisclosureOpen:
+      return EListStyleType::kDisclosureOpen;
+    case CSSValueID::kDisclosureClosed:
+      return EListStyleType::kDisclosureClosed;
     case CSSValueID::kDecimal:
       return EListStyleType::kDecimal;
     case CSSValueID::kDecimalLeadingZero:
@@ -369,6 +373,10 @@ inline CSSValueID PlatformEnumToCSSValueID(EListStyleType v) {
       return CSSValueID::kCircle;
     case EListStyleType::kSquare:
       return CSSValueID::kSquare;
+    case EListStyleType::kDisclosureOpen:
+      return CSSValueID::kDisclosureOpen;
+    case EListStyleType::kDisclosureClosed:
+      return CSSValueID::kDisclosureClosed;
     case EListStyleType::kDecimal:
       return CSSValueID::kDecimal;
     case EListStyleType::kDecimalLeadingZero:

third_party/blink/renderer/core/css/css_value_keywords.json5

@@ -435,6 +435,8 @@
     "disc",
     "circle",
     "square",
+    "disclosure-open",
+    "disclosure-closed",
     "decimal",
     "decimal-leading-zero",
     "arabic-indic",

third_party/blink/renderer/core/layout/layout_details_marker.cc

@@ -31,20 +31,21 @@ namespace blink {
 LayoutDetailsMarker::LayoutDetailsMarker(Element* element)
     : LayoutBlockFlow(element) {}
 
-LayoutDetailsMarker::Orientation LayoutDetailsMarker::GetOrientation() const {
-  NOT_DESTROYED();
+LayoutDetailsMarker::Orientation LayoutDetailsMarker::GetOrientation(
+    const ComputedStyle& style,
+    bool is_open) {
   // TODO(layout-dev): Sideways-lr and sideways-rl are not yet supported.
-  const auto mode = StyleRef().GetWritingMode();
+  const auto mode = style.GetWritingMode();
   DCHECK(mode != WritingMode::kSidewaysRl && mode != WritingMode::kSidewaysLr);
 
-  if (IsOpen()) {
-    if (mode == WritingMode::kHorizontalTb)
+  if (is_open) {
+    if (blink::IsHorizontalWritingMode(mode))
       return kDown;
-    return (mode == WritingMode::kVerticalRl) ? kLeft : kRight;
+    return IsFlippedBlocksWritingMode(mode) ? kLeft : kRight;
   }
-  if (mode == WritingMode::kHorizontalTb)
-    return StyleRef().IsLeftToRightDirection() ? kRight : kLeft;
-  return StyleRef().IsLeftToRightDirection() ? kDown : kUp;
+  if (blink::IsHorizontalWritingMode(mode))
+    return style.IsLeftToRightDirection() ? kRight : kLeft;
+  return style.IsLeftToRightDirection() ? kDown : kUp;
 }
 
 void LayoutDetailsMarker::Paint(const PaintInfo& paint_info) const {

third_party/blink/renderer/core/layout/layout_details_marker.h

@@ -31,7 +31,8 @@ class LayoutDetailsMarker final : public LayoutBlockFlow {
 
   enum Orientation { kUp, kDown, kLeft, kRight };
 
-  Orientation GetOrientation() const;
+  static Orientation GetOrientation(const ComputedStyle& style, bool is_open);
+  bool IsOpen() const;
 
   bool CreatesNewFormattingContext() const override {
     NOT_DESTROYED();
@@ -50,8 +51,6 @@ class LayoutDetailsMarker final : public LayoutBlockFlow {
            LayoutBlockFlow::IsOfType(type);
   }
   void Paint(const PaintInfo&) const override;
-
-  bool IsOpen() const;
 };
 
 template <>

third_party/blink/renderer/core/layout/layout_tree_as_text.cc

@@ -235,7 +235,8 @@ void LayoutTreeAsText::WriteLayoutObject(WTF::TextStream& ts,
 
   if (o.IsDetailsMarker()) {
     ts << ": ";
-    switch (To<LayoutDetailsMarker>(o).GetOrientation()) {
+    const auto& marker = To<LayoutDetailsMarker>(o);
+    switch (marker.GetOrientation(marker.StyleRef(), marker.IsOpen())) {
       case LayoutDetailsMarker::kLeft:
         ts << "left";
         break;

third_party/blink/renderer/core/layout/list_marker.cc

@@ -23,6 +23,15 @@ const int kCMarkerPaddingPx = 7;
 // Recommended UA margin for list markers.
 const int kCUAMarkerMarginEm = 1;
 
+namespace {
+
+LayoutUnit DisclosureSymbolSize(const FontMetrics& font_metrics) {
+  LayoutUnit font_height(font_metrics.Height());
+  return font_height * 2 / 3;
+}
+
+}  // namespace
+
 ListMarker::ListMarker() : marker_text_type_(kNotText) {}
 
 const ListMarker* ListMarker::Get(const LayoutObject* marker) {
@@ -281,6 +290,11 @@ LayoutUnit ListMarker::WidthOfSymbol(const ComputedStyle& style) {
   DCHECK(font_data);
   if (!font_data)
     return LayoutUnit();
+  const auto type = style.ListStyleType();
+  if (type == EListStyleType::kDisclosureOpen ||
+      type == EListStyleType::kDisclosureClosed) {
+    return 1 + DisclosureSymbolSize(font_data->GetFontMetrics()) + 1;
+  }
   return LayoutUnit((font_data->GetFontMetrics().Ascent() * 2 / 3 + 1) / 2 + 2);
 }
 
@@ -313,7 +327,8 @@ std::pair<LayoutUnit, LayoutUnit> ListMarker::InlineMarginsForOutside(
     margin_start = -marker_inline_size - kCMarkerPaddingPx;
     margin_end = LayoutUnit(kCMarkerPaddingPx);
   } else {
-    switch (GetListStyleCategory(list_item_style.ListStyleType())) {
+    auto type = list_item_style.ListStyleType();
+    switch (GetListStyleCategory(type)) {
       case ListStyleCategory::kNone:
         break;
       case ListStyleCategory::kSymbol: {
@@ -322,8 +337,11 @@ std::pair<LayoutUnit, LayoutUnit> ListMarker::InlineMarginsForOutside(
         if (!font_data)
           return {};
         const FontMetrics& font_metrics = font_data->GetFontMetrics();
-        int offset = font_metrics.Ascent() * 2 / 3;
-        margin_start = LayoutUnit(-offset - kCMarkerPaddingPx - 1);
+        LayoutUnit offset = (type == EListStyleType::kDisclosureOpen ||
+                             type == EListStyleType::kDisclosureClosed)
+                                ? DisclosureSymbolSize(font_metrics)
+                                : LayoutUnit(font_metrics.Ascent() * 2 / 3);
+        margin_start = -offset - kCMarkerPaddingPx - 1;
         margin_end = offset + kCMarkerPaddingPx + 1 - marker_inline_size;
         break;
       }
@@ -346,10 +364,18 @@ LayoutRect ListMarker::RelativeSymbolMarkerRect(const ComputedStyle& style,
   // TODO(wkorman): Review and clean up/document the calculations below.
   // http://crbug.com/543193
   const FontMetrics& font_metrics = font_data->GetFontMetrics();
-  int ascent = font_metrics.Ascent();
-  int bullet_width = (ascent * 2 / 3 + 1) / 2;
-  relative_rect = LayoutRect(1, 3 * (ascent - ascent * 2 / 3) / 2, bullet_width,
-                             bullet_width);
+  const auto type = style.ListStyleType();
+  if (type == EListStyleType::kDisclosureOpen ||
+      type == EListStyleType::kDisclosureClosed) {
+    LayoutUnit marker_size = DisclosureSymbolSize(font_metrics);
+    relative_rect =
+        LayoutRect(LayoutUnit(1), marker_size / 4, marker_size, marker_size);
+  } else {
+    int ascent = font_metrics.Ascent();
+    int bullet_width = (ascent * 2 / 3 + 1) / 2;
+    relative_rect = LayoutRect(1, 3 * (ascent - ascent * 2 / 3) / 2,
+                               bullet_width, bullet_width);
+  }
   if (!style.IsHorizontalWritingMode()) {
     relative_rect = relative_rect.TransposedRect();
     relative_rect.SetX(width - relative_rect.X() - relative_rect.Width());
@@ -367,6 +393,8 @@ ListMarker::ListStyleCategory ListMarker::GetListStyleCategory(
     case EListStyleType::kDisc:
     case EListStyleType::kCircle:
     case EListStyleType::kSquare:
+    case EListStyleType::kDisclosureOpen:
+    case EListStyleType::kDisclosureClosed:
       return ListStyleCategory::kSymbol;
     case EListStyleType::kArabicIndic:
     case EListStyleType::kArmenian:

third_party/blink/renderer/core/layout/list_marker_text.cc

@@ -461,6 +461,8 @@ static EListStyleType EffectiveListMarkerType(EListStyleType type, int count) {
     case EListStyleType::kDecimal:
     case EListStyleType::kDevanagari:
     case EListStyleType::kDisc:
+    case EListStyleType::kDisclosureClosed:
+    case EListStyleType::kDisclosureOpen:
     case EListStyleType::kGujarati:
     case EListStyleType::kGurmukhi:
     case EListStyleType::kKannada:
@@ -535,6 +537,8 @@ UChar Suffix(EListStyleType type, int count) {
   switch (effective_type) {
     case EListStyleType::kCircle:
     case EListStyleType::kDisc:
+    case EListStyleType::kDisclosureClosed:
+    case EListStyleType::kDisclosureOpen:
     case EListStyleType::kNone:
     case EListStyleType::kSquare:
       return ' ';
@@ -616,6 +620,14 @@ String GetText(EListStyleType type, int count) {
       return String(&kWhiteBulletCharacter, 1);
     case EListStyleType::kDisc:
       return String(&kBulletCharacter, 1);
+
+    // TODO(layout-dev): Text for disclosure-* should not depend on physical
+    // shape?
+    case EListStyleType::kDisclosureClosed:
+      return String(&kBlackRightPointingSmallTriangle, 1);
+    case EListStyleType::kDisclosureOpen:
+      return String(&kBlackDownPointingSmallTriangle, 1);
+
     case EListStyleType::kSquare:
       // The CSS 2.1 test suite uses U+25EE BLACK MEDIUM SMALL SQUARE
       // instead, but I think this looks better.

third_party/blink/renderer/core/paint/details_marker_painter.cc

@@ -78,8 +78,9 @@ static Path CreateRightArrowPath() {
   return CreatePath(points);
 }
 
-Path DetailsMarkerPainter::GetCanonicalPath() const {
-  switch (layout_details_marker_.GetOrientation()) {
+Path DetailsMarkerPainter::GetCanonicalPath(const ComputedStyle& style,
+                                            bool is_open) {
+  switch (LayoutDetailsMarker::GetOrientation(style, is_open)) {
     case LayoutDetailsMarker::kLeft:
       return CreateLeftArrowPath();
     case LayoutDetailsMarker::kRight:
@@ -94,7 +95,8 @@ Path DetailsMarkerPainter::GetCanonicalPath() const {
 }
 
 Path DetailsMarkerPainter::GetPath(const PhysicalOffset& origin) const {
-  Path result = GetCanonicalPath();
+  Path result = GetCanonicalPath(layout_details_marker_.StyleRef(),
+                                 layout_details_marker_.IsOpen());
   result.Transform(AffineTransform().Scale(
       layout_details_marker_.ContentWidth().ToFloat(),
       layout_details_marker_.ContentHeight().ToFloat()));

third_party/blink/renderer/core/paint/details_marker_painter.h

@@ -9,6 +9,7 @@
 
 namespace blink {
 
+class ComputedStyle;
 class Path;
 class LayoutDetailsMarker;
 struct PaintInfo;
@@ -22,9 +23,9 @@ class DetailsMarkerPainter {
       : layout_details_marker_(layout_details_marker) {}
 
   void Paint(const PaintInfo&);
+  static Path GetCanonicalPath(const ComputedStyle& style, bool is_open);
 
  private:
-  Path GetCanonicalPath() const;
   Path GetPath(const PhysicalOffset& origin) const;
 
   const LayoutDetailsMarker& layout_details_marker_;

third_party/blink/renderer/core/paint/list_marker_painter.cc

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/core/layout/list_marker_text.h"
 #include "third_party/blink/renderer/core/paint/box_model_object_painter.h"
 #include "third_party/blink/renderer/core/paint/box_painter.h"
+#include "third_party/blink/renderer/core/paint/details_marker_painter.h"
 #include "third_party/blink/renderer/core/paint/highlight_painting_utils.h"
 #include "third_party/blink/renderer/core/paint/paint_info.h"
 #include "third_party/blink/renderer/core/paint/scoped_paint_state.h"
@@ -48,6 +49,15 @@ void ListMarkerPainter::PaintSymbol(const PaintInfo& paint_info,
     case EListStyleType::kSquare:
       context.FillRect(marker);
       break;
+    case EListStyleType::kDisclosureOpen:
+    case EListStyleType::kDisclosureClosed: {
+      Path path = DetailsMarkerPainter::GetCanonicalPath(
+          style, style.ListStyleType() == EListStyleType::kDisclosureOpen);
+      path.Transform(AffineTransform().Scale(marker.Width(), marker.Height()));
+      path.Translate(FloatSize(marker.X(), marker.Y()));
+      context.FillPath(path);
+      break;
+    }
     default:
       NOTREACHED();
       break;

third_party/blink/renderer/core/style/computed_style_constants.h

@@ -282,9 +282,14 @@ inline ScrollbarGutter& operator|=(ScrollbarGutter& a, ScrollbarGutter b) {
 
 // https://drafts.csswg.org/css-counter-styles-3/#predefined-counters
 enum class EListStyleType : unsigned {
+  // https://drafts.csswg.org/css-counter-styles-3/#simple-symbolic
   kDisc,
   kCircle,
   kSquare,
+  kDisclosureOpen,
+  kDisclosureClosed,
+
+  // https://drafts.csswg.org/css-counter-styles-3/#simple-numeric
   kDecimal,
   kDecimalLeadingZero,
   kArabicIndic,
@@ -307,13 +312,18 @@ enum class EListStyleType : unsigned {
   kThai,
   kLowerRoman,
   kUpperRoman,
+
+  // https://drafts.csswg.org/css-counter-styles-3/#simple-alphabetic
   kLowerGreek,
   kLowerAlpha,
   kLowerLatin,
   kUpperAlpha,
   kUpperLatin,
+
+  // https://drafts.csswg.org/css-counter-styles-3/#simple-fixed
   kCjkEarthlyBranch,
   kCjkHeavenlyStem,
+
   kEthiopicHalehame,
   kEthiopicHalehameAm,
   kEthiopicHalehameTiEr,

third_party/blink/renderer/platform/wtf/text/character_names.h

@@ -44,6 +44,8 @@ const UChar kArabicLetterMarkCharacter = 0x061C;
 const UChar32 kArabicMathematicalOperatorMeemWithHahWithTatweel = 0x1EEF0;
 const UChar32 kArabicMathematicalOperatorHahWithDal = 0x1EEF1;
 const UChar kBlackCircleCharacter = 0x25CF;
+const UChar kBlackDownPointingSmallTriangle = 0x25BE;
+const UChar kBlackRightPointingSmallTriangle = 0x25B8;
 const UChar kBlackSquareCharacter = 0x25A0;
 const UChar kBlackUpPointingTriangleCharacter = 0x25B2;
 const UChar kBulletCharacter = 0x2022;
@@ -200,6 +202,8 @@ using WTF::unicode::kArabicLetterMarkCharacter;
 using WTF::unicode::kArabicMathematicalOperatorHahWithDal;
 using WTF::unicode::kArabicMathematicalOperatorMeemWithHahWithTatweel;
 using WTF::unicode::kBlackCircleCharacter;
+using WTF::unicode::kBlackDownPointingSmallTriangle;
+using WTF::unicode::kBlackRightPointingSmallTriangle;
 using WTF::unicode::kBlackSquareCharacter;
 using WTF::unicode::kBlackUpPointingTriangleCharacter;
 using WTF::unicode::kBulletCharacter;

third_party/blink/web_tests/external/wpt/css/css-lists/parsing/list-style-type-computed-expected.txt

@@ -3,6 +3,8 @@ PASS Property list-style-type value 'none'
 PASS Property list-style-type value 'disc'
 PASS Property list-style-type value 'circle'
 PASS Property list-style-type value 'square'
+PASS Property list-style-type value 'disclosure-open'
+PASS Property list-style-type value 'disclosure-closed'
 PASS Property list-style-type value 'decimal'
 PASS Property list-style-type value 'decimal-leading-zero'
 PASS Property list-style-type value 'lower-roman'

third_party/blink/web_tests/external/wpt/css/css-lists/parsing/list-style-type-computed.html

@@ -17,6 +17,8 @@ test_computed_value('list-style-type', 'none');
 test_computed_value('list-style-type', 'disc');
 test_computed_value('list-style-type', 'circle');
 test_computed_value('list-style-type', 'square');
+test_computed_value('list-style-type', 'disclosure-open');
+test_computed_value('list-style-type', 'disclosure-closed');
 test_computed_value('list-style-type', 'decimal');
 test_computed_value('list-style-type', 'decimal-leading-zero');
 test_computed_value('list-style-type', 'lower-roman');

third_party/blink/web_tests/external/wpt/css/css-lists/parsing/list-style-type-valid-expected.txt

@@ -3,6 +3,8 @@ PASS e.style['list-style-type'] = "none" should set the property value
 PASS e.style['list-style-type'] = "disc" should set the property value
 PASS e.style['list-style-type'] = "circle" should set the property value
 PASS e.style['list-style-type'] = "square" should set the property value
+PASS e.style['list-style-type'] = "disclosure-open" should set the property value
+PASS e.style['list-style-type'] = "disclosure-closed" should set the property value
 PASS e.style['list-style-type'] = "decimal" should set the property value
 PASS e.style['list-style-type'] = "decimal-leading-zero" should set the property value
 PASS e.style['list-style-type'] = "lower-roman" should set the property value

third_party/blink/web_tests/external/wpt/css/css-lists/parsing/list-style-type-valid.html

@@ -17,6 +17,8 @@ test_valid_value('list-style-type', 'none');
 test_valid_value('list-style-type', 'disc');
 test_valid_value('list-style-type', 'circle');
 test_valid_value('list-style-type', 'square');
+test_valid_value('list-style-type', 'disclosure-open');
+test_valid_value('list-style-type', 'disclosure-closed');
 test_valid_value('list-style-type', 'decimal');
 test_valid_value('list-style-type', 'decimal-leading-zero');
 test_valid_value('list-style-type', 'lower-roman');

third_party/blink/web_tests/external/wpt/css/cssom/serialize-values-expected.txt

 This is a testharness.js-based test.
-Found 678 tests; 677 PASS, 1 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 680 tests; 679 PASS, 1 FAIL, 0 TIMEOUT, 0 NOTRUN.
 PASS background-attachment: scroll
 PASS background-attachment: fixed
 PASS background-attachment: inherit
@@ -451,6 +451,8 @@ PASS list-style-position: inherit
 PASS list-style-type: disc
 PASS list-style-type: circle
 PASS list-style-type: square
+PASS list-style-type: disclosure-open
+PASS list-style-type: disclosure-closed
 PASS list-style-type: decimal
 PASS list-style-type: decimal-leading-zero
 PASS list-style-type: lower-roman

third_party/blink/web_tests/external/wpt/css/cssom/serialize-values.html

@@ -433,7 +433,8 @@
         'initial': 'outside',
       }],
       ['list-style-type', {
-        'values': ['disc', 'circle', 'square', 'decimal', 'decimal-leading-zero', 'lower-roman',
+        'values': ['disc', 'circle', 'square', 'disclosure-open', 'disclosure-closed',
+                   'decimal', 'decimal-leading-zero', 'lower-roman',
                    'upper-roman', 'lower-greek', 'lower-latin', 'upper-latin', 'armenian', 'georgian',
                    'lower-alpha', 'upper-alpha', 'none', 'inherit'],
         'initial': 'disc',

third_party/blink/web_tests/fast/lists/ol-display-types.html

@@ -23,6 +23,8 @@
 <li>Should be 14</li>
 <li style="list-style-type: circle;">Should have a circle</li>
 <li>Should be 16</li>
+<li style="list-style-type: disclosure-open;">Should have a triangle</li>
+<li style="list-style-type: disclosure-closed;">Should have another triangle</li>
 </ol>
 </body>
-</html>
+</html>
\ No newline at end of file