WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
45716
[BREWMP] Port unicode
https://bugs.webkit.org/show_bug.cgi?id=45716
Summary
[BREWMP] Port unicode
Kwang Yul Seo
Reported
2010-09-13 15:56:47 PDT
Port unicode.
Attachments
Patch
(10.26 KB, patch)
2010-09-13 16:07 PDT
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Patch
(13.79 KB, patch)
2010-09-13 16:08 PDT
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Patch
(13.79 KB, patch)
2010-09-13 16:55 PDT
,
Kwang Yul Seo
tkent
: review-
Details
Formatted Diff
Diff
Patch
(15.18 KB, patch)
2010-10-13 17:37 PDT
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Patch
(13.82 KB, patch)
2010-10-13 17:39 PDT
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Kwang Yul Seo
Comment 1
2010-09-13 16:07:12 PDT
Created
attachment 67485
[details]
Patch Brew MP port uses only the subset of ICU library to reduce the binary size. Follow the WinCE's implementation.
Kwang Yul Seo
Comment 2
2010-09-13 16:08:17 PDT
Created
attachment 67486
[details]
Patch Oops. Uploaded the wrong patch.
WebKit Review Bot
Comment 3
2010-09-13 16:42:50 PDT
Attachment 67486
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/unicode/brew/UnicodeBrew.h:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kwang Yul Seo
Comment 4
2010-09-13 16:55:39 PDT
Created
attachment 67490
[details]
Patch Fix style errors.
Kwang Yul Seo
Comment 5
2010-09-17 15:40:49 PDT
CC'ing Eric Seidel who reviewed
bug 27305
. This patch follows the WinCE approach.
Kent Tamura
Comment 6
2010-10-13 17:11:34 PDT
Comment on
attachment 67490
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=67490&action=review
r- because of style errors.
> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:84 > + int remainingCharacters = 0;
remainingCharacters should be declared just before it is used.
> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:85 > + if (sourceLength <= resultLength)
You should enclose this block with {} because the block contains two physical lines.
> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:88 > + else
ditto.
> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:101 > +int toUpper(UChar* result, int resultLength, const UChar* source, int sourceLength, bool* isError)
Same comments as toLower().
Kwang Yul Seo
Comment 7
2010-10-13 17:37:52 PDT
Created
attachment 70689
[details]
Patch Fix style errors.
Kwang Yul Seo
Comment 8
2010-10-13 17:39:02 PDT
Created
attachment 70690
[details]
Patch Oops. Remove duplicated ChangeLog entry.
Kent Tamura
Comment 9
2010-10-13 17:44:55 PDT
Comment on
attachment 70690
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=70690&action=review
> JavaScriptCore/wtf/unicode/brew/UnicodeBrew.cpp:94 > + remainingCharacters += sourceEnd - sourceIterator;
+= looks curious here. int remainingCharacters = sourceIterator < sourceEnd ? sourceEnd - sourceIterator : 0; would be better.
Kwang Yul Seo
Comment 10
2010-10-13 18:02:36 PDT
Manually committed
r69723
:
http://trac.webkit.org/changeset/69723
Kwang Yul Seo
Comment 11
2010-10-13 18:03:28 PDT
All reviewed patches have been landed. Closing bug.
Patrick R. Gansterer
Comment 12
2010-10-15 07:23:29 PDT
Do you think we can share the implementation between BREWMP and WinCE? Do you have an example of "UnicodeFromICU.h"? Is it a "copy" of ce_unicode.h|cpp? (I'm not very happy with the current WinCE implementation, maybe we can do it better) IMHO we can share the implementaions as a "UnicodeMinimalCopyOfICU". I don't think that there's something BREW specific in this files. PS: Use "svn cp" if you copy UnicodeBrew from UnicodeWinCE. This helps when looking at the history.
Kwang Yul Seo
Comment 13
2010-10-19 17:51:27 PDT
(In reply to
comment #12
)
> Do you think we can share the implementation between BREWMP and WinCE?
Yes, I just copied the implementation of WinCE only with minor style changes.
> Do you have an example of "UnicodeFromICU.h"? Is it a "copy" of ce_unicode.h|cpp? > (I'm not very happy with the current WinCE implementation, maybe we can do it better)
Yes, it is just a copy of ce_unicode.h|cpp with slight changes.
> > IMHO we can share the implementaions as a "UnicodeMinimalCopyOfICU". I don't think that there's something BREW specific in this files. >
I agree. There is no Brew MP specific code in these files.
> PS: Use "svn cp" if you copy UnicodeBrew from UnicodeWinCE. This helps when looking at the history.
Thanks for the tip.
Patrick R. Gansterer
Comment 14
2010-10-20 02:52:24 PDT
Created
bug 47977
for merging the unicode implementations.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug