Bug 52926

Summary:

Add segmentation test for range.expand

Product:

WebKit

Reporter:

Xiaomei Ji <xji>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ddkilzer, eric, jshin, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
layout test	rniwa: review-
layout test	none
layout test	none
layout test	rniwa: review+

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-	Details Formatted Diff Diff
layout test (7.36 KB, patch) 2011-01-24 13:31 PST, Xiaomei Ji	no flags	Details Formatted Diff Diff
layout test (7.53 KB, patch) 2011-01-24 14:00 PST, Xiaomei Ji	no flags	Details Formatted Diff Diff
layout test (7.70 KB, patch) 2011-01-24 14:17 PST, Xiaomei Ji	rniwa: review+	Details Formatted Diff Diff
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.