Changeset 83096 in webkit


Ignore:
Timestamp:
Apr 6, 2011 1:52:17 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-04-06 Naoki Takano <​takano.naoki@gmail.com>

Reviewed by David Levin.

Webkit ignores PgUp/PgDown/Home/End in SELECT tag objects
​https://bugs.webkit.org/show_bug.cgi?id=27658

  • fast/events/select-element-expected.txt: Added to check PgUp/PgDown/Home/End keys are working correctly in SELECT tag.
  • fast/events/select-element.html: Added for expectation.

2011-04-06 Naoki Takano <​takano.naoki@gmail.com>

Reviewed by David Levin.

Webkit ignores PgUp/PgDown/Home/End in SELECT tag objects
​https://bugs.webkit.org/show_bug.cgi?id=27658

Test: fast/events/select-element.html

  • dom/SelectElement.cpp: (WebCore::nextValidIndex): Moved from elsewhere in the file to be used by other routines. (WebCore::nextSelectableListIndexPageAway): Returns a selectable index one page away from the given index. (WebCore::nextSelectableListIndex): Implemented with nextValidIndex. And converted to a normal static function from a private function of SelectElement. (WebCore::previousSelectableListIndex): Implemented with nextValidIndex. And converted to a normal static function from a private function of SelectElement. (WebCore::firstSelectableListIndex): Returns the first selectable index. (WebCore::lastSelectableListIndex): Returns the last selectable index. (WebCore::SelectElement::menuListDefaultEventHandler): Converted from C cast to C++ cast. (WebCore::SelectElement::listBoxDefaultEventHandler): Adds support for PageUp/PageDown/Home/End with both single and multiple selection.
  • dom/SelectElement.h: (WebCore::SelectElement::): Remove nextSelectableListIndex() and previousSelectableListIndex().
  • rendering/RenderListBox.h: Makes RenderListBox::size public so that PageUp/PageDown behavior can use the actual list size rather than that specified in HTML. They can differ due to the minimum size imposed by RenderListBox.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r83094 r83096  
     12011-04-06  Naoki Takano  <takano.naoki@gmail.com>
     2
     3        Reviewed by David Levin.
     4
     5        Webkit ignores PgUp/PgDown/Home/End in SELECT tag objects
     6        https://bugs.webkit.org/show_bug.cgi?id=27658
     7
     8        * fast/events/select-element-expected.txt: Added to check PgUp/PgDown/Home/End keys are working correctly in SELECT tag.
     9        * fast/events/select-element.html: Added for expectation.
     10
    1112011-04-06  Adrienne Walker  <enne@google.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r83095 r83096  
     12011-04-06  Naoki Takano  <takano.naoki@gmail.com>
     2
     3        Reviewed by David Levin.
     4
     5        Webkit ignores PgUp/PgDown/Home/End in SELECT tag objects
     6        https://bugs.webkit.org/show_bug.cgi?id=27658
     7
     8        Test: fast/events/select-element.html
     9
     10        * dom/SelectElement.cpp:
     11        (WebCore::nextValidIndex): Moved from elsewhere in the file to be used by other routines.
     12        (WebCore::nextSelectableListIndexPageAway): Returns a selectable index one page away from the given index.
     13        (WebCore::nextSelectableListIndex): Implemented with nextValidIndex.
     14        And converted to a normal static function from a private function of SelectElement.
     15        (WebCore::previousSelectableListIndex): Implemented with nextValidIndex.
     16        And converted to a normal static function from a private function of SelectElement.
     17        (WebCore::firstSelectableListIndex): Returns the first selectable index.
     18        (WebCore::lastSelectableListIndex): Returns the last selectable index.
     19        (WebCore::SelectElement::menuListDefaultEventHandler): Converted from C cast to C++ cast.
     20        (WebCore::SelectElement::listBoxDefaultEventHandler): Adds support for PageUp/PageDown/Home/End with both single and multiple selection.
     21
     22        * dom/SelectElement.h:
     23        (WebCore::SelectElement::): Remove nextSelectableListIndex() and previousSelectableListIndex().
     24
     25        * rendering/RenderListBox.h: Makes RenderListBox::size public so that PageUp/PageDown behavior can use the actual list size rather than that specified in HTML.
     26        They can differ due to the minimum size imposed by RenderListBox.
     27
    1282011-04-06  David Hyatt  <hyatt@apple.com>
    229

  • trunk/Source/WebCore/dom/SelectElement.cpp

    r79762 r83096  
    7171static const DOMTimeStamp typeAheadTimeout = 1000;
    7272
     73enum SkipDirection {
     74    SkipBackwards = -1,
     75    SkipForwards = 1
     76};
     77
     78// Returns the 1st valid item |skip| items from |listIndex| in direction |direction| if there is one.
     79// Otherwise, it returns the valid item closest to that boundary which is past |listIndex| if there is one.
     80// Otherwise, it returns |listIndex|.
     81// Valid means that it is enabled and an option element.
     82static int nextValidIndex(const Vector<Element*>& listItems, int listIndex, SkipDirection direction, int skip)
     83{
     84    ASSERT(direction == -1 || direction == 1);
     85    int lastGoodIndex = listIndex;
     86    int size = listItems.size();
     87    for (listIndex += direction; listIndex >= 0 && listIndex < size; listIndex += direction) {
     88        --skip;
     89        if (!listItems[listIndex]->disabled() && isOptionElement(listItems[listIndex])) {
     90            lastGoodIndex = listIndex;
     91            if (skip <= 0)
     92                break;
     93        }
     94    }
     95    return lastGoodIndex;
     96}
     97
     98static int nextSelectableListIndex(SelectElementData& data, Element* element, int startIndex)
     99{
     100    return nextValidIndex(data.listItems(element), startIndex, SkipForwards, 1);
     101}
     102
     103static int previousSelectableListIndex(SelectElementData& data, Element* element, int startIndex)
     104{
     105    if (startIndex == -1)
     106        startIndex = data.listItems(element).size();
     107    return nextValidIndex(data.listItems(element), startIndex, SkipBackwards, 1);
     108}
     109
     110static int firstSelectableListIndex(SelectElementData& data, Element* element)
     111{
     112    const Vector<Element*>& items = data.listItems(element);
     113    int index = nextValidIndex(items, items.size(), SkipBackwards, INT_MAX);
     114    if (static_cast<unsigned>(index) == items.size())
     115        return -1;
     116    return index;
     117}
     118
     119static int lastSelectableListIndex(SelectElementData& data, Element* element)
     120{
     121    return nextValidIndex(data.listItems(element), -1, SkipForwards, INT_MAX);
     122}
     123
     124// Returns the index of the next valid item one page away from |startIndex| in direction |direction|.
     125static int nextSelectableListIndexPageAway(SelectElementData& data, Element* element, int startIndex, SkipDirection direction)
     126{
     127    const Vector<Element*>& items = data.listItems(element);
     128    // Can't use data->size() because renderer forces a minimum size.
     129    int pageSize = 0;
     130    if (element->renderer()->isListBox())
     131        pageSize = toRenderListBox(element->renderer())->size() - 1; // -1 so we still show context
     132
     133    // One page away, but not outside valid bounds.
     134    // If there is a valid option item one page away, the index is chosen.
     135    // If there is no exact one page away valid option, returns startIndex or the most far index.
     136    int edgeIndex = (direction == SkipForwards) ? 0 : (items.size() - 1);
     137    int skipAmount = pageSize + ((direction == SkipForwards) ? startIndex : (edgeIndex - startIndex));
     138    return nextValidIndex(items, edgeIndex, direction, skipAmount);
     139}
     140
    73141void SelectElement::selectAll(SelectElementData& data, Element* element)
    74142{
    … …  
    103171        lastOnChangeSelection.append(optionElement && optionElement->selected());
    104172    }
    105 }
    106 
    107 int SelectElement::nextSelectableListIndex(SelectElementData& data, Element* element, int startIndex)
    108 {
    109     const Vector<Element*>& items = data.listItems(element);
    110     int index = startIndex + 1;
    111     while (index >= 0 && (unsigned) index < items.size() && (!isOptionElement(items[index]) || items[index]->disabled()))
    112         ++index;
    113     if ((unsigned) index == items.size())
    114         return startIndex;
    115     return index;
    116 }
    117 
    118 int SelectElement::previousSelectableListIndex(SelectElementData& data, Element* element, int startIndex)
    119 {
    120     const Vector<Element*>& items = data.listItems(element);
    121     if (startIndex == -1)
    122         startIndex = items.size();
    123     int index = startIndex - 1;
    124     while (index >= 0 && (unsigned) index < items.size() && (!isOptionElement(items[index]) || items[index]->disabled()))
    125         --index;
    126     if (index == -1)
    127         return startIndex;
    128     return index;
    129173}
    130174
    … …  
    515559    setOptionsChangedOnRenderer(data, element);
    516560    element->setNeedsStyleRecalc();
    517 }
    518    
    519 enum SkipDirection {
    520     SkipBackwards = -1,
    521     SkipForwards = 1
    522 };
    523 
    524 // Returns the index of the next valid list item |skip| items past |listIndex| in direction |direction|.
    525 static int nextValidIndex(const Vector<Element*>& listItems, int listIndex, SkipDirection direction, int skip)
    526 {
    527     int lastGoodIndex = listIndex;
    528     int size = listItems.size();
    529     for (listIndex += direction; listIndex >= 0 && listIndex < size; listIndex += direction) {
    530         --skip;
    531         if (!listItems[listIndex]->disabled() && isOptionElement(listItems[listIndex])) {
    532             lastGoodIndex = listIndex;
    533             if (skip <= 0)
    534                 break;
    535         }
    536     }
    537     return lastGoodIndex;
    538561}
    539562
    … …  
    595618            handled = true;
    596619        }
    597        
    598         if (handled && listIndex >= 0 && (unsigned)listIndex < listItems.size())
     620
     621        if (handled && listIndex >= 0 && static_cast<unsigned>(listIndex) < listItems.size())
    599622            setSelectedIndex(data, element, listToOptionIndex(data, element, listIndex));
    600623
    … …  
    761784        const String& keyIdentifier = static_cast<KeyboardEvent*>(event)->keyIdentifier();
    762785
    763         int endIndex = 0;       
     786        bool handled = false;
     787        int endIndex = 0;
    764788        if (data.activeSelectionEndIndex() < 0) {
    765789            // Initialize the end index
    766             if (keyIdentifier == "Down")
    767                 endIndex = nextSelectableListIndex(data, element, lastSelectedListIndex(data, element));
    768             else if (keyIdentifier == "Up")
    769                 endIndex = previousSelectableListIndex(data, element, optionToListIndex(data, element, selectedIndex(data, element)));
     790            if (keyIdentifier == "Down" || keyIdentifier == "PageDown") {
     791                int startIndex = lastSelectedListIndex(data, element);
     792                handled = true;
     793                if (keyIdentifier == "Down")
     794                    endIndex = nextSelectableListIndex(data, element, startIndex);
     795                else
     796                    endIndex = nextSelectableListIndexPageAway(data, element, startIndex, SkipForwards);
     797            } else if (keyIdentifier == "Up" || keyIdentifier == "PageUp") {
     798                int startIndex = optionToListIndex(data, element, selectedIndex(data, element));
     799                handled = true;
     800                if (keyIdentifier == "Up")
     801                    endIndex = previousSelectableListIndex(data, element, startIndex);
     802                else
     803                    endIndex = nextSelectableListIndexPageAway(data, element, startIndex, SkipBackwards);
     804            }
    770805        } else {
    771806            // Set the end index based on the current end index
    772             if (keyIdentifier == "Down")
     807            if (keyIdentifier == "Down") {
    773808                endIndex = nextSelectableListIndex(data, element, data.activeSelectionEndIndex());
    774             else if (keyIdentifier == "Up")
    775                 endIndex = previousSelectableListIndex(data, element, data.activeSelectionEndIndex());   
     809                handled = true;
     810            } else if (keyIdentifier == "Up") {
     811                endIndex = previousSelectableListIndex(data, element, data.activeSelectionEndIndex());
     812                handled = true;
     813            } else if (keyIdentifier == "PageDown") {
     814                endIndex = nextSelectableListIndexPageAway(data, element, data.activeSelectionEndIndex(), SkipForwards);
     815                handled = true;
     816            } else if (keyIdentifier == "PageUp") {
     817                endIndex = nextSelectableListIndexPageAway(data, element, data.activeSelectionEndIndex(), SkipBackwards);
     818                handled = true;
     819            }
     820        }
     821        if (keyIdentifier == "Home") {
     822            endIndex = firstSelectableListIndex(data, element);
     823            handled = true;
     824        } else if (keyIdentifier == "End") {
     825            endIndex = lastSelectableListIndex(data, element);
     826            handled = true;
    776827        }
    777828
    … …  
    781832                return;
    782833
    783         if (keyIdentifier == "Down" || keyIdentifier == "Up") {
     834        if (endIndex >= 0 && handled) {
    784835            // Save the selection so it can be compared to the new selection when dispatching change events immediately after making the new selection.
    785836            saveLastSelection(data, element);
    786837
    787             ASSERT_UNUSED(listItems, !listItems.size() || (endIndex >= 0 && (unsigned) endIndex < listItems.size()));
     838            ASSERT_UNUSED(listItems, !listItems.size() || (endIndex >= 0 && static_cast<unsigned>(endIndex) < listItems.size()));
    788839            setActiveSelectionEndIndex(data, endIndex);
    789            
     840
    790841            bool selectNewItem = !data.multiple() || static_cast<KeyboardEvent*>(event)->shiftKey() || !isSpatialNavigationEnabled(element->document()->frame());
    791842            if (selectNewItem)

  • trunk/Source/WebCore/dom/SelectElement.h

    r73606 r83096  
    7373    static void selectAll(SelectElementData&, Element*);
    7474    static void saveLastSelection(SelectElementData&, Element*);
    75     static int nextSelectableListIndex(SelectElementData&, Element*, int startIndex);
    76     static int previousSelectableListIndex(SelectElementData&, Element*, int startIndex);
    7775    static void setActiveSelectionAnchorIndex(SelectElementData&, Element*, int index);
    7876    static void setActiveSelectionEndIndex(SelectElementData&, int index);

  • trunk/Source/WebCore/rendering/RenderListBox.h

    r81704 r83096  
    5353
    5454    int scrollToward(const IntPoint&); // Returns the new index or -1 if no scroll occurred
     55
     56    int size() const;
    5557
    5658private:
    … …  
    126128    int itemHeight() const;
    127129    void valueChanged(unsigned listIndex);
    128     int size() const;
    129130    int numVisibleItems() const;
    130131    int numItems() const;
Note: See TracChangeset for help on using the changeset viewer.