[webkit-reviews] review denied: [Bug 22784] [Chromium] Home and End buttons do not do anything in drop down lists : [Attachment 32580] 2nd patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 10 14:46:03 PDT 2009


Mark Rowe (bdash) <mrowe at apple.com> has denied Jens Alfke <snej at google.com>'s
request for review:
Bug 22784: [Chromium] Home and End buttons do not do anything in drop down
lists
https://bugs.webkit.org/show_bug.cgi?id=22784

Attachment 32580: 2nd patch
https://bugs.webkit.org/attachment.cgi?id=32580&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
> Index: WebCore/dom/SelectElement.cpp
> ===================================================================
> --- WebCore/dom/SelectElement.cpp	(revision 45713)
> +++ WebCore/dom/SelectElement.cpp	(working copy)
> @@ -47,7 +47,10 @@
>  #include "WMLSelectElement.h"
>  #endif
>  
> -#if PLATFORM(MAC)
> +// On Mac OS, pop up the menu if the user presses the arrow keys while a
Select element has focus.
> +// On other platforms, just change the selection.
> +// (Cannot just test PLATFORM(MAC), because that is false in Chromium due to
its AppKit assumptions.)
> +#if PLATFORM(DARWIN) && !PLATFORM(GTK) && !PLATFORM(WX)
>  #define ARROW_KEYS_POP_MENU 1
>  #else
>  #define ARROW_KEYS_POP_MENU 0

At the very least this is not correct because it omits Qt.  Thinking about this
a little more, I suspect what you really want is PLATFORM(MAC) ||
(PLATFORM(CHROMIUM) && PLATFORM(DARWIN))

Marking as r- due to this and the other unaddressed comments.


More information about the webkit-reviews mailing list