[Webkit-unassigned] [Bug 121990] AX: Regression: media controls are no longer accessible

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #215736|review?                     |review+
               Flag|                            |




--- Comment #9 from Darin Adler <darin at apple.com>  2013-11-01 11:32:39 PST ---
(From update of attachment 215736)
View in context: https://bugs.webkit.org/attachment.cgi?id=215736&action=review

Looks great.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:80
> +    /* Localized strings */

We normally use // comments everywhere, not /* */ comments.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:81
> +    loc: function(s){

Is this a standard name we use for this function elsewhere? Could we use a word rather than a little abbreviation? I understand that brevity is nice, but I at least find words easier to read.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:85
> +            return s; // TODO: log something if string not localized.

We normally use FIXME for all things like this, never TODO.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:91
> +        'Display Fullscreen': 'Display Fullscreen',

We use two words for this in Apple user interface and marketing materials, “Full Screen”, not one word “Fullscreen”.

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