Bug 1530594 - treat menulists as always being leaf frames. r=tnikkel a=lizzard
authorNeil Deakin <neil@mozilla.com>
Sat, 09 Mar 2019 05:35:44 +0200
changeset 513459 0bc584b08a5f13eef5686c63cca8bfd3698b8c6e
parent 513458 fb07e38d917f7d605c23dec973b5811b9b9bd9a9
child 513460 c4d0a83454a291d01e5c8d2f9215a2d43c890562
push id10859
push userbtara@mozilla.com
push dateSat, 09 Mar 2019 04:18:49 +0000
treeherdermozilla-beta@93d78ef4e591 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, lizzard
bugs1530594
milestone66.0
Bug 1530594 - treat menulists as always being leaf frames. r=tnikkel a=lizzard Summary: ...false from IsLeafDynamic anyway, the only effect is that this one menulist is now not a leaf, allowing its content to be generated upfront. This particular menulist only has two items in it anyway so there shouldn't be a performance issue, r?tnikkel Reviewers: tnikkel Reviewed By: tnikkel Bug #: 1530594 Differential Revision: https://phabricator.services.mozilla.com/D22517
layout/xul/nsMenuPopupFrame.cpp
toolkit/content/tests/chrome/test_menulist_keynav.xul
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -406,39 +406,28 @@ bool nsMenuPopupFrame::IsLeafDynamic() c
     // is always generated right away.
     return !mContent->AsElement()->HasAttr(kNameSpaceID_None, nsGkAtoms::type);
   }
 
   // menu popups generate their child frames lazily only when opened, so
   // behave like a leaf frame. However, generate child frames normally if the
   // following is true:
   //
-  // - If the parent menu is a <menulist> unless its sizetopopup is set to
-  // "none".
+  // - If the parent menu is a <menulist>
   // - For other elements, if the parent menu has a sizetopopup attribute.
   //
   // In these cases the size of the parent menu is dependent on the size of
   // the popup, so the frames need to exist in order to calculate this size.
   nsIContent* parentContent = mContent->GetParent();
   if (!parentContent) {
     return false;
   }
 
   if (parentContent->IsXULElement(nsGkAtoms::menulist)) {
-    Element* parent = parentContent->AsElement();
-    if (!parent->HasAttr(kNameSpaceID_None, nsGkAtoms::sizetopopup)) {
-      // No prop set, generate child frames normally for the
-      // default value ("pref").
-      return false;
-    }
-
-    nsAutoString sizedToPopup;
-    parent->GetAttr(kNameSpaceID_None, nsGkAtoms::sizetopopup, sizedToPopup);
-    // Don't generate child frame only if the property is set to none.
-    return sizedToPopup.EqualsLiteral("none");
+    return false;
   }
 
   return (!parentContent->IsElement() ||
           !parentContent->AsElement()->HasAttr(kNameSpaceID_None,
                                                nsGkAtoms::sizetopopup));
 }
 
 void nsMenuPopupFrame::UpdateWidgetProperties() {
--- a/toolkit/content/tests/chrome/test_menulist_keynav.xul
+++ b/toolkit/content/tests/chrome/test_menulist_keynav.xul
@@ -22,16 +22,24 @@
 <menulist id="list2">
   <menupopup id="popup" onpopupshown="checkCursorNavigation();">
     <menuitem id="b1" label="One"/>
     <menuitem id="b2" label="Two" selected="true"/>
     <menuitem id="b3" label="Three"/>
     <menuitem id="b4" label="Four"/>
   </menupopup>
 </menulist>
+<menulist id="list3" sizetopopup="none">
+  <menupopup>
+    <menuitem id="s1" label="One"/>
+    <menuitem id="s2" label="Two"/>
+    <menuitem id="s3" label="Three"/>
+    <menuitem id="s4" label="Four"/>
+  </menupopup>
+</menulist>
 
 <script class="testbody" type="application/javascript">
 <![CDATA[
 
 SimpleTest.waitForExplicitFinish();
 
 var gShowPopup = false;
 var gModifiers = 0;
@@ -270,16 +278,26 @@ function checkCursorNavigation()
 
   synthesizeKey("KEY_ArrowUp", {altKey: true});
   is(list.open, ismac, "alt+up closes popup");
 
   if (ismac) {
     list.open = false;
   }
 
+  // Finally, test a menulist with sizetopopup="none" to ensure keyboard navigation
+  // still works when the popup has not been opened.
+  if (!ismac) {
+    let unsizedMenulist = document.getElementById("list3");
+    unsizedMenulist.focus();
+    synthesizeKey("KEY_ArrowDown");
+    is(unsizedMenulist.selectedIndex, 1, "correct menulist index on keydown");
+    is(unsizedMenulist.label, "Two", "correct menulist label on keydown");
+  }
+
   SimpleTest.finish();
 }
 
 SimpleTest.waitForFocus(runTests);
 
 ]]>
 </script>