[webkit-reviews] review canceled: [Bug 97899] Clean up Localizer-related functions : [Attachment 166227] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 28 06:05:54 PDT 2012


Kent Tamura <tkent at chromium.org> has canceled Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 97899: Clean up Localizer-related functions
https://bugs.webkit.org/show_bug.cgi?id=97899

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=166227&action=review


>> Source/WebCore/ChangeLog:12
>> +	    - Add Localizer::createDefault to improvde code readability
> 
> Nit: improvde => improve

will fix

>> Source/WebKit/blackberry/WebCoreSupport/ColorPickerClient.cpp:88
>> +	return m_element->document()->getCachedLocalizer(nullAtom);
> 
> Given that you're introducing Element::localizer() as a short path for
document()->getCachedLocalizer(computeInheritedLanguage()), you might also want
to introduce a short path for
m_element->document()->getCachedLocalizer(nullAtom). Maybe you can introduce
Element::localizer(const AtomicString& locale = nullAtom)?

I object it because a Localizer with nullAtom is not a object associated to
this Element.
However, adding the default argument to getCachedLocalizer() would make sense.


More information about the webkit-reviews mailing list