[webkit-reviews] review granted: [Bug 72715] Implement AccessKeyLabel attribute. : [Attachment 400330] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Noam Rosenthal <noam at webkit.org>'s
request for review:
Bug 72715: Implement AccessKeyLabel attribute.
https://bugs.webkit.org/show_bug.cgi?id=72715

Attachment 400330: Patch

https://bugs.webkit.org/attachment.cgi?id=400330&action=review




--- Comment #41 from Darin Adler <darin at apple.com> ---
Comment on attachment 400330
  --> https://bugs.webkit.org/attachment.cgi?id=400330
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400330&action=review

Is this the correct string, even for unusual keys on the keyboard?

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

Looked this up in the Unicode character database: Should be upArrowhead. The
word "arrowhead" is one word.

> Source/WebCore/html/HTMLElement.cpp:730
> +    const AtomString& accessKey =
attributeWithoutSynchronization(accesskeyAttr);

I suggest auto& here.

> Source/WebCore/html/HTMLElement.cpp:731
> +    if (accessKey.isEmpty() || accessKey.isNull())

Null strings are also empty; no need to check both.

> Source/WebCore/html/HTMLElement.cpp:737
> +    OptionSet<PlatformEvent::Modifier> modifiers =
EventHandler::accessKeyModifiers();

I suggest auto here.