[webkit-reviews] review granted: [Bug 35993] [Qt] Should not access key modifier be the Alt key? : [Attachment 50490] Proposed patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Diego Gonzalez
<diego.gonzalez at openbossa.org>'s request for review:
Bug 35993: [Qt] Should not access key modifier be the Alt key?
https://bugs.webkit.org/show_bug.cgi?id=35993

Attachment 50490: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=50490&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
Looks good to me. Please fix the changelog before landing though. Atl -> Alt
and
I don't think the main title should be a question :)

> From b44596d47f9c1531843b7feae8adc058f6f0a01d Mon Sep 17 00:00:00 2001
> From: Diego Gonzalez <diego.gonzalez at openbossa.org>
> Date: Wed, 10 Mar 2010 19:16:26 -0400
> Subject: [PATCH] Return Atl Key for access key modifiers in Qt EventHandler
> 
> LayoutTests:
>     fast/forms/access-key.html
>     fast/forms/focus-selection-textarea.html
>     fast/events/access-key-self-destruct.html
>     fast/forms/legend-access-key.html
> ---
>  LayoutTests/ChangeLog	      |   17 +++++++++++++++++
>  LayoutTests/platform/qt/Skipped    |    4 ----
>  WebCore/ChangeLog		      |   18 ++++++++++++++++++
>  WebCore/page/qt/EventHandlerQt.cpp |    6 +++++-
>  4 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index f1c4010..74a3301 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,20 @@
> +2010-03-10  Diego Gonzalez  <diego.gonzalez at openbossa.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Return Atl Key for access key modifiers in Qt EventHandler
> +
> +	   LayoutTests:
> +	       fast/forms/access-key.html
> +	       fast/forms/focus-selection-textarea.html
> +	       fast/events/access-key-self-destruct.html
> +	       fast/forms/legend-access-key.html
> +
> +	   [Qt] Should not access key modifier be the Alt key?
> +	   https://bugs.webkit.org/show_bug.cgi?id=35993
> +
> +	   * platform/qt/Skipped:
> +
>  2010-03-05  Dimitri Glazkov	<dglazkov at chromium.org>
>  
>	   Reviewed by Sam Weinig.
> diff --git a/LayoutTests/platform/qt/Skipped
b/LayoutTests/platform/qt/Skipped
> index 8399bcf..c3a9863 100644
> --- a/LayoutTests/platform/qt/Skipped
> +++ b/LayoutTests/platform/qt/Skipped
> @@ -461,12 +461,10 @@ fast/events/standalone-image-drag-to-editable.html
>  fast/events/updateLayoutForHitTest.html
>  fast/forms/input-list.html
>  fast/forms/input-selectedoption.html
> -fast/forms/access-key.html
>  fast/forms/button-cannot-be-nested.html
>  fast/forms/control-restrict-line-height.html
>  fast/forms/drag-into-textarea.html
>  fast/forms/focus-selection-input.html
> -fast/forms/focus-selection-textarea.html
>  fast/forms/input-readonly-autoscroll.html
>  fast/forms/input-text-click-outside.html
>  fast/forms/input-text-double-click.html
> @@ -1157,7 +1155,6 @@ fast/dynamic/selection-highlight-adjust.html
>  fast/encoding/char-decoding.html
>  fast/encoding/frame-default-enc.html
>  fast/encoding/invalid-xml.html
> -fast/events/access-key-self-destruct.html
>  fast/events/attempt-scroll-with-no-scrollbars.html
>  fast/events/key-events-in-input-button.html
>  fast/forms/input-align-image.html
> @@ -1165,7 +1162,6 @@ fast/forms/input-align.html
>  fast/forms/input-double-click-selection-gap-bug.html
>  fast/forms/input-radio-checked-tab.html
>  fast/forms/input-text-option-delete.html
> -fast/forms/legend-access-key.html
>  fast/forms/textarea-rows-cols.html
>  fast/frames/onlyCommentInIFrame.html
>  fast/inline/positionedLifetime.html
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index eef9d52..87319d3 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,21 @@
> +2010-03-10  Diego Gonzalez  <diego.gonzalez at openbossa.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Return Atl Key for access key modifiers in Qt EventHandler
> +
> +	   LayoutTests:
> +	       fast/forms/access-key.html
> +	       fast/forms/focus-selection-textarea.html
> +	       fast/events/access-key-self-destruct.html
> +	       fast/forms/legend-access-key.html
> +
> +	   [Qt] Should not access key modifier be the Alt key?
> +	   https://bugs.webkit.org/show_bug.cgi?id=35993
> +
> +	   * page/qt/EventHandlerQt.cpp:
> +	   (WebCore::EventHandler::accessKeyModifiers):
> +
>  2010-03-10  Jeremy Orlow  <jorlow at chromium.org>
>  
>	   Reviewed by Darin Fisher.
> diff --git a/WebCore/page/qt/EventHandlerQt.cpp
b/WebCore/page/qt/EventHandlerQt.cpp
> index 3425289..d7982fa 100644
> --- a/WebCore/page/qt/EventHandlerQt.cpp
> +++ b/WebCore/page/qt/EventHandlerQt.cpp
> @@ -132,7 +132,11 @@ bool
EventHandler::passMouseReleaseEventToSubframe(MouseEventWithHitTestResults&
>  
>  unsigned EventHandler::accessKeyModifiers()
>  {
> -    return PlatformKeyboardEvent::CtrlKey;
> +#if OS(DARWIN)
> +    return PlatformKeyboardEvent::CtrlKey | PlatformKeyboardEvent::AltKey;
> +#else
> +    return PlatformKeyboardEvent::AltKey;
> +#endif
>  }
>  
>  }
> -- 
> 1.6.3.3
>