Update <select> when any of its <option> children has "display: none"

Source/core/rendering/RenderListBox.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  *               2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 101 matching lines @@
 }
 
 inline HTMLSelectElement* RenderListBox::selectElement() const
 {
     return toHTMLSelectElement(node());
 }
 
 void RenderListBox::updateFromElement()
 {
     FontCachePurgePreventer fontCachePurgePreventer;
-
     if (m_optionsChanged) {
         const Vector<HTMLElement*>& listItems = selectElement()->listItems();
-        int size = numItems();
+        int size = listItems.size();
 
         float width = 0;
+
+        m_listItemIndex.clear();
         for (int i = 0; i < size; ++i) {
             HTMLElement* element = listItems[i];
+            RenderStyle* listItemStyle = element->renderStyle();
+            if (listItemStyle && listItemStyle->display() == NONE)
+                continue;
+
+            m_listItemIndex.append(i);
             String text;
             Font itemFont = style()->font();
             if (isHTMLOptionElement(*element)) {
                 text = toHTMLOptionElement(*element).textIndentedToRespectGroupLabel();
             } else if (isHTMLOptGroupElement(*element)) {
                 text = toHTMLOptGroupElement(*element).groupLabelText();
                 FontDescription d = itemFont.fontDescription();
                 d.setWeight(d.bolderWeight());
                 itemFont = Font(d);
                 itemFont.update(document().styleEngine()->fontSelector());
@@ skipping 55 matching lines @@
         scrollToRevealSelection();
     }
 }
 
 void RenderListBox::scrollToRevealSelection()
 {
     HTMLSelectElement* select = selectElement();
 
     m_scrollToRevealSelectionAfterLayout = false;
 
-    int firstIndex = select->activeSelectionStartListIndex();
-    if (firstIndex >= 0 && !listIndexIsVisible(select->activeSelectionEndListIndex()))
+    int firstIndex = static_cast<int>(m_listItemIndex.find(select->activeSelectionStartListIndex()));
+    int lastIndex = static_cast<int>(m_listItemIndex.find(select->activeSelectionEndListIndex()));

[esprehn, 2014/03/21 20:43:39]

Why can't we just iterate the set of items and only consider the visible ones?

[spartha, 2014/03/21 21:29:03]

As against using the vector? We can do that I suppose. But then we would have to
loop through the item list each time we need to get the element from the display
order (which may not be the same as the item list in Select). It comes down to
the choice between speed v/s space. I have yet to come across select with huge
number of items. So if space is a concern I can try the other approach.
Thoughts? 

+    if (firstIndex >= 0 && !listIndexIsVisible(lastIndex))
         scrollToRevealElementAtListIndex(firstIndex);
 }
 
 void RenderListBox::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const
 {
     maxLogicalWidth = m_optionsWidth + 2 * optionsSpacingHorizontal;
     if (m_vBar)
         maxLogicalWidth += verticalScrollbarWidth();
     if (!style()->width().isPercent())
         minLogicalWidth = maxLogicalWidth;
@@ skipping 38 matching lines @@
 }
 
 int RenderListBox::numVisibleItems() const
 {
     // Only count fully visible rows. But don't return 0 even if only part of a row shows.
     return max<int>(1, (contentHeight() + rowSpacing) / itemHeight());
 }
 
 int RenderListBox::numItems() const
 {
-    return selectElement()->listItems().size();
+    return m_listItemIndex.size();
 }
 
 LayoutUnit RenderListBox::listHeight() const
 {
     return itemHeight() * numItems() - rowSpacing;
 }
 
 void RenderListBox::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
 {
     LayoutUnit height = itemHeight() * size() - rowSpacing + borderAndPaddingHeight();
@@ skipping 68 matching lines @@
     int selectedItem = select->activeSelectionEndListIndex();
     if (selectedItem >= 0) {
         rects.append(pixelSnappedIntRect(itemBoundingBoxRect(additionalOffset, selectedItem)));
         return;
     }
 
     // No selected items, find the first non-disabled item.
     int size = numItems();
     const Vector<HTMLElement*>& listItems = select->listItems();
     for (int i = 0; i < size; ++i) {
-        HTMLElement* element = listItems[i];
+        HTMLElement* element = listItems[m_listItemIndex[i]];

[esprehn, 2014/03/21 20:43:39]

ditto.

         if (isHTMLOptionElement(*element) && !element->isDisabledFormControl()) {
             rects.append(pixelSnappedIntRect(itemBoundingBoxRect(additionalOffset, i)));
             return;
         }
     }
 }
 
 int RenderListBox::scrollbarLeft() const
 {
     int scrollbarLeft = 0;
@@ skipping 36 matching lines @@
     return offset;
 }
 
 void RenderListBox::paintItemForeground(PaintInfo& paintInfo, const LayoutPoint& paintOffset, int listIndex)
 {
     FontCachePurgePreventer fontCachePurgePreventer;
 
     HTMLSelectElement* select = selectElement();
 
     const Vector<HTMLElement*>& listItems = select->listItems();
-    HTMLElement* element = listItems[listIndex];
+    HTMLElement* element = listItems[m_listItemIndex[listIndex]];
 
     RenderStyle* itemStyle = element->renderStyle();
     if (!itemStyle)
         itemStyle = style();
 
     if (itemStyle->visibility() == HIDDEN)
         return;
 
     String itemText;
     bool isOptionElement = isHTMLOptionElement(*element);
@@ skipping 28 matching lines @@
 
     // Draw the item text
     TextRunPaintInfo textRunPaintInfo(textRun);
     textRunPaintInfo.bounds = r;
     paintInfo.context->drawBidiText(itemFont, textRunPaintInfo, roundedIntPoint(r.location()));
 }
 
 void RenderListBox::paintItemBackground(PaintInfo& paintInfo, const LayoutPoint& paintOffset, int listIndex)
 {
     const Vector<HTMLElement*>& listItems = selectElement()->listItems();
-    HTMLElement* element = listItems[listIndex];
+    HTMLElement* element = listItems[m_listItemIndex[listIndex]];
 
     Color backColor;
     if (isHTMLOptionElement(*element) && ((toHTMLOptionElement(*element).selected() && selectElement()->suggestedIndex() < 0) || listIndex == selectElement()->suggestedIndex())) {
         if (frame()->selection().isFocusedAndActive() && document().focusedElement() == node())
             backColor = RenderTheme::theme().activeListBoxSelectionBackgroundColor();
         else
             backColor = RenderTheme::theme().inactiveListBoxSelectionBackgroundColor();
     } else {
         backColor = element->renderStyle() ? resolveColor(element->renderStyle(), CSSPropertyBackgroundColor) : resolveColor(CSSPropertyBackgroundColor);
     }
@@ skipping 111 matching lines @@
 
     int endIndex = scrollToward(pos);
     if (selectElement()->isDisabledFormControl())
         return;
 
     if (endIndex >= 0) {
         HTMLSelectElement* select = selectElement();
         m_inAutoscroll = true;
 
         if (!select->multiple())
-            select->setActiveSelectionAnchorIndex(endIndex);
+            select->setActiveSelectionAnchorIndex(m_listItemIndex[endIndex]);
 
-        select->setActiveSelectionEndIndex(endIndex);
+        select->setActiveSelectionEndIndex(m_listItemIndex[endIndex]);
         select->updateListBoxSelection(!select->multiple());
         m_inAutoscroll = false;
     }
 }
 
 void RenderListBox::stopAutoscroll()
 {
     if (selectElement()->isDisabledFormControl())
         return;
 
@@ skipping 104 matching lines @@
 }
 
 bool RenderListBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction hitTestAction)
 {
     if (!RenderBlockFlow::nodeAtPoint(request, result, locationInContainer, accumulatedOffset, hitTestAction))
         return false;
     const Vector<HTMLElement*>& listItems = selectElement()->listItems();
     int size = numItems();
     LayoutPoint adjustedLocation = accumulatedOffset + location();
 
     for (int i = 0; i < size; ++i) {

[esprehn, 2014/03/21 20:43:39]

Since we're walking all of them anyway, why can't we just skip display: none
ones?

         if (itemBoundingBoxRect(adjustedLocation, i).contains(locationInContainer.point())) {
-            if (Element* node = listItems[i]) {
+            if (Element* node = listItems[m_listItemIndex[i]]) {
                 result.setInnerNode(node);
                 if (!result.innerNonSharedNode())
                     result.setInnerNonSharedNode(node);
                 result.setLocalPoint(locationInContainer.point() - toLayoutSize(adjustedLocation));
                 break;
             }
         }
     }
 
     return true;
@@ skipping 211 matching lines @@
 
     if (m_vBar)
         m_vBar->styleChanged();
 
     // Force an update since we know the scrollbars have changed things.
     if (document().hasAnnotatedRegions())
         document().setAnnotatedRegionsDirty(true);
 }
 
 } // namespace WebCore