Option Groups with display: none should not show the children option elements

Source/core/rendering/RenderMenuList.cpp

Index: Source/core/rendering/RenderMenuList.cpp
diff --git a/Source/core/rendering/RenderMenuList.cpp b/Source/core/rendering/RenderMenuList.cpp
index 57ac3b845c3a872b4546cb08d9fab7f77e7551d6..36c686863a272208f8c3b4771df2f0498673a17c 100644
--- a/Source/core/rendering/RenderMenuList.cpp
+++ b/Source/core/rendering/RenderMenuList.cpp
@@ -502,8 +502,18 @@ PopupMenuStyle RenderMenuList::itemStyle(unsigned listIndex) const
     getItemBackgroundColor(listIndex, itemBackgroundColor, itemHasCustomBackgroundColor);
 
     RenderStyle* style = element->renderStyle() ? element->renderStyle() : element->computedStyle();
+    bool displayNone = style && style->display() == NONE;

[keishi, 2014/08/01 02:01:11]

This nested display:none case is already handled correctly for listboxes, so we
just need to fix the menu list popup.

So I think this change to RenderMenuList.cpp should be all you need. You
shouldn't need to change HTMLSelectElement.cpp and HTMLOptionElement.cpp.

But there is a problem on Mac. ExternalPopupMenu ignores
PopupMenuStyle::isDisplayNone. We would need to fix that too.

[spartha, 2014/08/01 09:29:16]

Thanks for the review! Even I felt that it would be sufficient. But the
RenderMenuList change will affect only when the popup is shown. But the test
case in the patch will fail without the HTMLSelectElement.cpp and
HTMLOptionElement.cpp changes. The test case just sets the focus on the select
and uses keyup/down to select an option. 
I haven't tried the patch on Mac. Let me try it on the bots first (I don't have
access to a Mac :( )

+    if (!displayNone) {
+        HTMLElement* parent = Traversal<HTMLElement>::firstAncestor(*element);
+        ASSERT(parent);
+        if (parent->hasTagName(optgroupTag)) {
+            RenderStyle* parentStyle = parent->renderStyle() ? parent->renderStyle() : parent->computedStyle();
+            if (parentStyle && parentStyle->display() == NONE)
+                displayNone = true;
+        }
+    }
     return style ? PopupMenuStyle(resolveColor(style, CSSPropertyColor), itemBackgroundColor, style->font(), style->visibility() == VISIBLE,
-        style->display() == NONE, style->textIndent(), style->direction(), isOverride(style->unicodeBidi()),
+        displayNone, style->textIndent(), style->direction(), isOverride(style->unicodeBidi()),
         itemHasCustomBackgroundColor ? PopupMenuStyle::CustomBackgroundColor : PopupMenuStyle::DefaultBackgroundColor) : menuStyle();
 }