[webkit-reviews] review denied: [Bug 72715] Implement AccessKeyLabel attribute. : [Attachment 118992] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied  review:
Bug 72715: Implement AccessKeyLabel attribute.
https://bugs.webkit.org/show_bug.cgi?id=72715

Attachment 118992: Patch
https://bugs.webkit.org/attachment.cgi?id=118992&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118992&action=review


> Source/WebCore/html/HTMLElement.cpp:822
> +    if (oldAccessKeyModifierValue != accessKeyModifierValue) {
> +	   oldAccessKeyModifierValue = accessKeyModifierValue;
> +#if PLATFORM(MAC)
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::CtrlKey)
> +	       accessKeyLabelValue.append(upArrowhead);
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::AltKey)
> +	       accessKeyLabelValue.append(optionKey);
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::ShiftKey)
> +	       accessKeyLabelValue.append(upwardsWhiteArrow);
> +#else
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::CtrlKey)
> +	       accessKeyLabelValue += "Ctrl+";
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::AltKey)
> +	       accessKeyLabelValue += "Alt+";
> +	   if (accessKeyModifierValue & PlatformKeyboardEvent::ShiftKey)
> +	       accessKeyLabelValue += "Shift+";
> +#endif
> +    }

There’s an obvious bug here where if you keep doing this with different
modifier combinations, the prefix will keep getting longer and longer. If you
alternatively call this for Control A and Shift A you will eventually get
Ctrl+Shift+Ctrl+Shift+A.