[Webkit-unassigned] [Bug 27658] Webkit ignores PgUp/PgDown/Home/End in SELECT tag objects

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27658





--- Comment #27 from Naoki Takano <takano.naoki at gmail.com>  2011-04-04 13:57:17 PST ---
David,

Thank you for your review.

I can finish the revise tonight.

Hope the next is the last round;-)

(In reply to comment #26)
> (From update of attachment 87998 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=87998&action=review
> 
> Nicely done!
> 
> We're in the final stretch and the next patch may very well be the last.
> 
> Thanks for getting this into shape and addressing all feedback well!
> 
> > LayoutTests/fast/events/select-element.html:165
> > +function testPageDownWithDisabledElements() {
> 
> Why is there no test in this function start starts at 0 or 7?
> 
> > LayoutTests/fast/events/select-element.html:173
> > +function testPageUpWithDisabledElements() {
> 
> Why is there no test in this function start starts at 0 or 7?
> 
> > Source/WebCore/dom/SelectElement.cpp:134
> > +    int edgeIndex = (direction== SkipForwards) ? 0 : (items.size() - 1);
> 
> Need a space after direction.
> 
> > Source/WebCore/dom/SelectElement.cpp:789
> >                  endIndex = nextSelectableListIndex(data, element, lastSelectedListIndex(data, element));
> 
> For down, the start index is lastSelectedListIndex(data, element)
> For up, the start index is optionToListIndex(data, element, selectedIndex(data, element))
> 
> Consider pulling this out of the clauses:
> 
> if (keyIdentifier == "Down" || keyIdentifier == "PageDown") {
>     int startIndex = lastSelectedListIndex(data, element)
>     handled = true;
>     if (keyIdentifier == "Down")
>         endIndex = nextSelectableListIndex(data, element, startIndex);
>     else
>         endIndex = nextSelectableListIndexPageAway(data, element, startIndex, SkipForwards);    
> } else if (keyIdentifier == "Up" || keyIdentifier == "PageUp")  {
>     int startIndex = optionToListIndex(data, element, selectedIndex(data, element))
>     handled = true;
>     ... similar to above...
> }

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.