[Webkit-unassigned] [Bug 15816] Cannot select multiple non-adjacent items in a multiple select control with the keyboard only

[bugzilla-daemon at webkit.org]

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





--- Comment #21 from Mario Sanchez Prada <msanchez at igalia.com>  2010-12-13 01:31:01 PST ---
(In reply to comment #20)
> (From update of attachment 73584 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=73584&action=review
> 
> > WebCore/dom/SelectElement.cpp:785
> > +            bool isCtrlKey = data.multiple() && static_cast<KeyboardEvent*>(event)->metaKey();
> 
> This variable has a pretty confusing name, since it doesn't just track whether the control key is down or not.

My humble "casual review" :-):

What about naming it something like 'keepCurrentSelection' ? After reading the patch I think its purpose si to know precisely when we don't want to change the current selection despite of moving down or up through the list, so I guess that would be a good name.

Other than that, I think this patch lacks some comments in the ChangeLogs in order to qualify for the r+

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