Bug 1499127 - Only consider <option>/<optgroup> elements that has a frame when determining the default row size, and use a fallback value if there are none. r=emilio
authorMats Palmgren <mats@mozilla.com>
Tue, 16 Oct 2018 16:35:04 +0200
changeset 499958 abaa52cda0ad84656583a260f33fb64fe569a4ef
parent 499957 c96e54bae30c098a4b10a42721bf58295a1409f7
child 499959 64e46a7cc29ff5e5a3afa19b50150fb4035645ae
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1499127
milestone64.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 1499127 - Only consider <option>/<optgroup> elements that has a frame when determining the default row size, and use a fallback value if there are none. r=emilio
layout/forms/nsListControlFrame.cpp
testing/web-platform/meta/css/css-display/select-4-option-optgroup-display-none.html.ini
testing/web-platform/tests/css/css-display/select-4-option-optgroup-display-none-ref.html
testing/web-platform/tests/css/css-display/select-4-option-optgroup-display-none.html
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -274,60 +274,64 @@ NS_QUERYFRAME_TAIL_INHERITING(nsHTMLScro
 #ifdef ACCESSIBILITY
 a11y::AccType
 nsListControlFrame::AccessibleType()
 {
   return a11y::eHTMLSelectListType;
 }
 #endif
 
-static nscoord
-GetMaxOptionBSize(nsIFrame* aContainer, WritingMode aWM)
+// Return true if we found at least one <option> or non-empty <optgroup> label
+// that has a frame.  aResult will be the maximum BSize of those.
+static bool
+GetMaxRowBSize(nsIFrame* aContainer, WritingMode aWM, nscoord* aResult)
 {
-  nscoord result = 0;
-  for (nsIFrame* option : aContainer->PrincipalChildList()) {
-    nscoord optionBSize;
-    if (HTMLOptGroupElement::FromNode(option->GetContent())) {
-      // An optgroup; drill through any scroll frame and recurse.  |frame| might
-      // be null here though if |option| is an anonymous leaf frame of some sort.
-      auto frame = option->GetContentInsertionFrame();
-      optionBSize = frame ? GetMaxOptionBSize(frame, aWM) : 0;
+  bool found = false;
+  for (nsIFrame* child : aContainer->PrincipalChildList()) {
+    if (child->GetContent()->IsHTMLElement(nsGkAtoms::optgroup)) {
+      // An optgroup; drill through any scroll frame and recurse.  |inner| might
+      // be null here though if |inner| is an anonymous leaf frame of some sort.
+      auto inner = child->GetContentInsertionFrame();
+      if (inner && GetMaxRowBSize(inner, aWM, aResult)) {
+        found = true;
+      }
     } else {
-      // an option
-      optionBSize = option->BSize(aWM);
+      // an option or optgroup label
+      bool isOptGroupLabel = child->Style()->IsPseudoElement() &&
+        aContainer->GetContent()->IsHTMLElement(nsGkAtoms::optgroup);
+      nscoord childBSize = child->BSize(aWM);
+      // XXX bug 1499176: skip empty <optgroup> labels (zero bsize) for now
+      if (!isOptGroupLabel || childBSize > nscoord(0)) {
+        found = true;
+        *aResult = std::max(childBSize, *aResult);
+      }
     }
-    if (result < optionBSize)
-      result = optionBSize;
   }
-  return result;
+  return found;
 }
 
 //-----------------------------------------------------------------
 // Main Reflow for ListBox/Dropdown
 //-----------------------------------------------------------------
 
 nscoord
 nsListControlFrame::CalcBSizeOfARow()
 {
   // Calculate the block size in our writing mode of a single row in the
   // listbox or dropdown list by using the tallest thing in the subtree,
   // since there may be option groups in addition to option elements,
   // either of which may be visible or invisible, may use different
   // fonts, etc.
-  int32_t blockSizeOfARow = GetMaxOptionBSize(GetOptionsContainer(),
-                                              GetWritingMode());
-
-  // Check to see if we have zero items (and optimize by checking
-  // blockSizeOfARow first)
-  if (blockSizeOfARow == 0 && GetNumberOfOptions() == 0) {
+  nscoord rowBSize(0);
+  if (!GetMaxRowBSize(GetOptionsContainer(), GetWritingMode(), &rowBSize)) {
+    // We don't have any <option>s or <optgroup> labels with a frame.
     float inflation = nsLayoutUtils::FontSizeInflationFor(this);
-    blockSizeOfARow = CalcFallbackRowBSize(inflation);
+    rowBSize = CalcFallbackRowBSize(inflation);
   }
-
-  return blockSizeOfARow;
+  return rowBSize;
 }
 
 nscoord
 nsListControlFrame::GetPrefISize(gfxContext *aRenderingContext)
 {
   nscoord result;
   DISPLAY_PREF_INLINE_SIZE(this, result);
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-display/select-4-option-optgroup-display-none.html.ini
@@ -0,0 +1,2 @@
+[select-4-option-optgroup-display-none.html]
+  expected: FAIL
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/select-4-option-optgroup-display-none-ref.html
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html><head>
+  <meta charset="utf-8">
+  <title>Reference: display:none on OPTION and OPTGROUP</title>
+  <link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+  <style>
+
+.none { display:none; }
+.contents { display:contents; }
+.red { color: red; }
+.green { color: green; }
+
+select { -webkit-appearance: none; }
+
+  </style>
+</head>
+<body>
+
+<pre>FAIL if there is any red color</pre>
+
+<optgroup></optgroup>
+<optgroup class="contents red"></optgroup>
+<optgroup class="contents green" label="optgroup"></optgroup>
+
+<br>
+
+<select class="red" size="4"></select>
+<select size="4" class="red"></select>
+<select size="4" class="red"></select>
+<select size="4" class="red"><optgroup></select>
+<select size="4"></select>
+<select size="4" class="red"></select>
+<select size="4" class="red"></select>
+<select size="4" class="red"></select>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-display/select-4-option-optgroup-display-none.html
@@ -0,0 +1,48 @@
+<!DOCTYPE HTML>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html><head>
+  <meta charset="utf-8">
+  <title>CSS Test: display:none on OPTION and OPTGROUP</title>
+  <link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+  <link rel="help" href="https://drafts.csswg.org/css-display-3/#valdef-display-none">
+  <link rel="match" href="select-4-option-optgroup-display-none-ref.html">
+  <style>
+
+.none { display:none; }
+.contents { display:contents; }
+.red { color: red; }
+.green { color: green; }
+
+select { -webkit-appearance: none; }
+
+  </style>
+</head>
+<body>
+
+<pre>FAIL if there is any red color</pre>
+
+<option class="none red">text</option>
+<optgroup class="none red">text</optgroup>
+
+<optgroup class="none red"><option>option</option></optgroup>
+<optgroup><option class="none red">option</option></optgroup>
+<optgroup class="contents red"><option class="none">option</option></optgroup>
+<optgroup class="contents green" label="optgroup"><option class="none red">option</option></optgroup>
+<optgroup class="none red" label="optgroup"><option class="red">option</option></optgroup>
+
+<br>
+
+<select class="red" size="4">select</select>
+<select size="4" class="red"><optgroup class="none" label="optgroup"></select>
+<select size="4" class="red"><option class="none">option</select>
+<select size="4" class="red"><optgroup><option class="none">option</select>
+<select size="4"><optgroup class="none"><option class="green">option</select>
+<select size="4" class="red"><optgroup class="none green" label="optgroup"><option>option</select>
+<select size="4" class="red"><optgroup class="none"><option class="none">option</select>
+<select size="4" class="red"><optgroup class="none green" label="optgroup"><option class="none">option</select>
+
+</body>
+</html>