[webkit-reviews] review granted: [Bug 54260] HTML5 <details> and <summary>: localized text : [Attachment 82083] patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Luiz Agostini
<luiz at webkit.org>'s request for review:
Bug 54260: HTML5 <details> and <summary>: localized text
https://bugs.webkit.org/show_bug.cgi?id=54260

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=82083&action=review

You should also run Tools/Scripts/update-webkit-localizable-strings to update
the Localizable.strings file used by the Mac and Windows ports.

> Source/WebCore/ChangeLog:9
> +	   The method summaryDefaultLabel was added to LocalizationStrategy
class and to
> +	   platform/LocalizedStrings. It is used to provide the default label
to be used by a

The name "summaryDefaultLabel" to me sounds like it is "the default label used
for a <summary> element". Clearly that is wrong. I also think that "label" is
mostly (only?) used to refer to the text inside (or associated with) a button.
So maybe "defaultDetailsSummaryText" would be clearer?

> Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.h:61
>      virtual String fileButtonNoFileSelectedLabel();
> +    virtual WTF::String summaryDefaultLabel();

Please omit "WTF::" here to match the other functions.