[Webkit-unassigned] [Bug 73586] Upstream text codec and web string files of BlackBerry API

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Leo Yang <leo.yang at torchmobile.com.cn>  2011-12-08 23:52:36 PST ---
(From update of attachment 117484)
View in context: https://bugs.webkit.org/attachment.cgi?id=117484&action=review

Informal review.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:28
> +#include "Vector.h"

#include <wtf/Vector.h> instead of #include "Vector.h".

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:36
> +namespace BlackBerry {
> +namespace WebKit {
> +
> +

I would keep only one empty line here.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:47
> +    TextEncoding te(encoding);

I would use textEncoding instead of te.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:57
> +    WTF::String enc(encoding);

WTF:: is not needed, should be removed. And I would change enc to encodingName.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:68
> +
> +

I would remove an extra empty line.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:71
> +    TextEncoding teSource(sourceEncoding);

I would change teSource to textEncodingSource.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:75
> +    TextEncoding teTarget(targetEncoding);

I would name the variable textEncodingTarget.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:82
> +    WTF::String ucs2 = codecSource.decode(sourceStart, sourceLength, true, true, sawError);

Remove WTF::.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:89
> +    WTF::CString encoded = codecTarget.encode(ucs2.characters(), ucs2.length(), WebCore::EntitiesForUnencodables);

Ditto.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:108
> +    WTF::Vector<char> vect;

Remove WTF::. I would rename vect to something like result.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:119
> +    WTF::Vector<char> vect;

Ditto.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:132
> +    WTF::String escapedString(escaped.data(), escaped.length());

Remove WTF::.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:133
> +

I would remove this empty line.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:135
> +    WTF::String urlString = WebCore::decodeURLEscapeSequences(escapedString);
> +    WTF::CString utf8 = urlString.utf8();

Remove WTF::.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:143
> +    WTF::String urlString(url.data(), url.length());

Remove WTF::.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:144
> +

I would remove this empty line.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:146
> +    WTF::String escapedString = WebCore::encodeWithURLEscapeSequences(urlString);
> +    WTF::CString utf8 = escapedString.utf8();

Remove WTF::.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:156
> +    WTF::String mimeString(mime.data(), mime.length());
> +
> +    WTF::String ext = WebCore::MIMETypeRegistry::getPreferredExtensionForMIMEType(mimeString);

Remove WTF::. I would change ext to extension.

> Source/WebKit/blackberry/Api/WebKitTextCodec.cpp:160
> +    WTF::CString utf8 = ext.utf8();

Remove WTF::.

> Source/WebKit/blackberry/Api/WebString.cpp:26
> +using WTF::String;

Could this line be removed?

> Source/WebKit/blackberry/Api/WebString.cpp:31
> +WebString::WebString(const char* latin1)
> +: m_impl(static_cast<WebStringImpl*>(WTF::StringImpl::create(latin1).releaseRef()))

You should add 4 spaces before the colon.

> Source/WebKit/blackberry/Api/WebString.cpp:36
> +: m_impl(static_cast<WebStringImpl*>(WTF::StringImpl::create(latin1, length).releaseRef()))

Ditto.

> Source/WebKit/blackberry/Api/WebString.cpp:41
> +: m_impl(static_cast<WebStringImpl*>(WTF::StringImpl::create(utf16, length).releaseRef()))

Ditto.

> Source/WebKit/blackberry/Api/WebString.cpp:59
> +: m_impl(str.m_impl)

Ditto.

> Source/WebKit/blackberry/Api/WebString.cpp:67
> +    return WTF::String::fromUTF8(utf8);

Remove WTF::.

> Source/WebKit/blackberry/Api/WebString.cpp:87
> +    WTF::String str(m_impl);

Ditto.

> Source/WebKit/blackberry/Api/WebString.h:44
> +    WebString& operator=(const WebString&);
> +
> +    std::string utf8() const;
> +
> +    static WebString fromUtf8(const char* utf8);
> +

I would remove extra empty lines.

> Source/WebKit/blackberry/Api/WebString.h:48
> +

Ditto.

> Source/WebKit/blackberry/Api/WebString.h:51
> +

Ditto.

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