RESOLVED FIXED 52926
Add segmentation test for range.expand 
https://bugs.webkit.org/show_bug.cgi?id=52926
Summary Add segmentation test for range.expand
Xiaomei Ji
Reported 2011-01-21 14:45:26 PST
Add segmentation test for range.expand.
Attachments
layout test (5.86 KB, patch)
2011-01-21 14:49 PST, Xiaomei Ji 		rniwa: review-
DetailsFormatted DiffDiff
layout test (7.36 KB, patch)
2011-01-24 13:31 PST, Xiaomei Ji 		no flags
DetailsFormatted DiffDiff
layout test (7.53 KB, patch)
2011-01-24 14:00 PST, Xiaomei Ji 		no flags
DetailsFormatted DiffDiff
layout test (7.70 KB, patch)
2011-01-24 14:17 PST, Xiaomei Ji 		rniwa: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Xiaomei Ji
Comment 1 2011-01-21 14:49:24 PST
Created attachment 79798 [details] layout test
Ryosuke Niwa
Comment 2 2011-01-21 14:58:39 PST
Comment on attachment 79798 [details] layout test This is going to break other platforms. We should either be adding chromium specific test or should be adding a new test when Xiaomei lands a patch to upstream fixes.
Jungshik Shin
Comment 3 2011-01-21 15:22:47 PST
Just some clarification: ICU bug to upstream Chrome's CJK segmentation patch is http://bugs.icu-project.org/trac/ticket/2229 Even if that's added to the ICU trunk today, it'll be a year or more before Apple picks it up in next version of Mac OS X. Moreover, there are webkit ports that do not use ICU (e.g. Qt, GTK). So fixing the above ICU bug wouldn't help them.
Jungshik Shin
Comment 4 2011-01-21 15:31:42 PST
BTW, shouldn't the test be more 'interesting' (dealing with non-trivial but fairly clear-cut cases)? 国务院公布《国有土地) 国务院 公布 ( 国有 土地 )
Xiaomei Ji
Comment 5 2011-01-24 13:31:28 PST
Created attachment 79966 [details] layout test
Xiaomei Ji
Comment 6 2011-01-24 14:00:11 PST
Created attachment 79973 [details] layout test
Ryosuke Niwa
Comment 7 2011-01-24 14:03:38 PST
Comment on attachment 79973 [details] layout test View in context: https://bugs.webkit.org/attachment.cgi?id=79973&action=review > LayoutTests/platform/chromium/fast/text/international/cjk-segmentation-expected.txt:3 > +Test Chinese Segmentation. > + > + We should print out PASS at the end or otherwise, or otherwise it's hard to tell if the test really passed or not.
Ryosuke Niwa
Comment 8 2011-01-24 14:04:54 PST
Comment on attachment 79973 [details] layout test View in context: https://bugs.webkit.org/attachment.cgi?id=79973&action=review > LayoutTests/fast/text/international/cjk-segmentation.html:2 > +<html> > +<head> Missing DOCTYPE here: <!DOCTYPE html> > LayoutTests/fast/text/international/cjk-segmentation.html:3 > +<META HTTP-EQUIV="CONTENT-TYPE" CONTENT="text/html; charset=UTF-8"> We probably want to use lowercases for tag and attribute names for the consistency. > LayoutTests/fast/text/international/cjk-segmentation.html:21 > + } Maybe print PASS here?
Xiaomei Ji
Comment 9 2011-01-24 14:17:14 PST
Created attachment 79976 [details] layout test
Ryosuke Niwa
Comment 10 2011-01-24 14:20:41 PST
Comment on attachment 79976 [details] layout test View in context: https://bugs.webkit.org/attachment.cgi?id=79976&action=review > LayoutTests/fast/text/international/cjk-segmentation.html:5 > +<title>Test for CJK segmentation.</title> Nit: Don't put period in the title. > LayoutTests/fast/text/international/cjk-segmentation.html:50 > + if (pass == true) { > + log("ALL PASS"); > + } Nit: no { } around a single line statement.
Xiaomei Ji
Comment 11 2011-01-24 14:27:00 PST
Committed r76548: <http://trac.webkit.org/changeset/76548>
WebKit Review Bot
Comment 12 2011-01-24 14:55:49 PST
http://trac.webkit.org/changeset/76548 might have broken Qt Linux Release The following tests are not passing: fast/text/international/cjk-segmentation.html
Ryosuke Niwa
Comment 13 2011-01-24 16:03:14 PST
This test is failing on Chromium mac port.
David Kilzer (:ddkilzer)
Comment 14 2011-09-30 13:33:24 PDT
(In reply to comment #3) > Just some clarification: > > ICU bug to upstream Chrome's CJK segmentation patch is http://bugs.icu-project.org/trac/ticket/2229 > > Even if that's added to the ICU trunk today, it'll be a year or more before Apple picks it up in next version of Mac OS X. No patch has ever been posted to that ICU bug. Could you at least post a WIP patch so someone else can work on it if you don't have time?
Note You need to log in before you can comment on or make changes to this bug.