[Webkit-unassigned] [Bug 72715] Implement AccessKeyLabel attribute.

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Alexey Proskuryakov <ap at webkit.org>  2011-11-28 08:59:10 PST ---
(From update of attachment 116719)
View in context: https://bugs.webkit.org/attachment.cgi?id=116719&action=review

> Source/JavaScriptCore/wtf/unicode/CharacterNames.h:67
> +#if OS(DARWIN)

These shouldn't be in ifdefs.

> Source/JavaScriptCore/wtf/unicode/CharacterNames.h:84
> +const UChar upArrowHead = 0x2303;

Should be "upArrowhead" to match Unicode name.

> Source/JavaScriptCore/wtf/unicode/CharacterNames.h:85
> +const UChar upWardWhiteArrow = 0x21E7;

Should be "upwardsWhiteArrow".

> Source/WebCore/html/HTMLElement.cpp:785
> +    String accessKeyValue = getAttribute(accesskeyAttr);

Per HTML5, accesskey attribute can list multiple keys, which the engine will choose from. We should use assigned access key here, not the whole attribute string.

> Source/WebCore/html/HTMLElement.cpp:791
> +    static String accessKeyLabelValue;
> +    if (accessKeyLabelValue.isNull()) {

Several issues here.

1. accessKeyLabelValue will not be cached when null (on platforms that don't support access keys).

2. It will not be updated when EventHandler::accessKeyModifiers() changes. That can happen in practice, see e.g. version in EventHandlerMac.mm.

> Source/WebCore/html/HTMLElement.cpp:793
> +#if OS(DARWIN)

Matching OS X menu style does not sound like an OS(DARWIN) thing. But I'm not sure what would be a better match.

> Source/WebCore/html/HTMLElement.cpp:813
> +    return String(accessKeyLabelValue + accessKeyValue);

Why did you need to explicitly construct a String to return?

I think that key itself should be converted to upper case - the regular way to represent shortcuts is Alt+A or ⌥A, not Alt+a and ⌥a.

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