Bug 47645 - Conversion to text test: editing/spelling/spellcheck-attribute.html
Summary: Conversion to text test: editing/spelling/spellcheck-attribute.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on: 47659
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-13 19:14 PDT by Hajime Morrita
Modified: 2010-10-22 03:18 PDT (History)
5 users (show)

See Also:


Attachments
Patch (33.03 KB, patch)
2010-10-13 20:47 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (73.76 KB, patch)
2010-10-22 01:10 PDT, Hajime Morrita
tkent: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2010-10-13 19:14:59 PDT
Should convert a pixel test to text based test.
Comment 1 Hajime Morrita 2010-10-13 20:47:15 PDT
Created attachment 70705 [details]
Patch
Comment 2 Kent Tamura 2010-10-13 20:51:23 PDT
Comment on attachment 70705 [details]
Patch

ok
Comment 3 Ryosuke Niwa 2010-10-13 20:54:20 PDT
> LayoutTests/editing/spelling/spellcheck-attribute.html:37
> +    if (window.textInputController) {
> +        var marked = (0 != textInputController.hasSpellingMarker(0, 2));
> +        if (shouldMarked == marked)
> +           log("PASS:" + id);
> +        else
> +           log("FAIL:" + id);
> +    }

This would fail on all platforms that do not implement textInputController.  Are you sure all platforms that currently run this test implement textInputController?  We can't do this conversion if the answer is no because we'll be losing the test coverage then.
Comment 4 Kent Tamura 2010-10-13 21:06:21 PDT
Comment on attachment 70705 [details]
Patch

win/gtk/qt has no textInputController.hasSpelingMarkers(), and we should remove expectation images for Chromium, and so on.
Comment 5 Kent Tamura 2010-10-13 21:08:55 PDT
(In reply to comment #3)
> This would fail on all platforms that do not implement textInputController.  Are you sure all platforms that currently run this test implement textInputController?  We can't do this conversion if the answer is no because we'll be losing the test coverage then.

GTK and Qt is skipping this test.  So we can convert this if Windows port gets textInputController.hasSpellingMarkers().
Comment 6 Ryosuke Niwa 2010-10-13 21:15:32 PDT
(In reply to comment #1)
> Created an attachment (id=70705) [details]
> Patch

You might find my script posted on https://bugs.webkit.org/show_bug.cgi?id=45100 useful.  It lets you delete all png, checksum, etc... under LayoutTest/platform/
Comment 7 Hajime Morrita 2010-10-13 22:31:47 PDT
(In reply to comment #6)
> You might find my script posted on https://bugs.webkit.org/show_bug.cgi?id=45100 useful.  It lets you delete all png, checksum, etc... under LayoutTest/platform/
Thanks! I didn't noticed this.
Comment 8 Hajime Morrita 2010-10-22 01:10:33 PDT
Created attachment 71534 [details]
Patch
Comment 9 Hajime Morrita 2010-10-22 01:13:29 PDT
> GTK and Qt is skipping this test.  So we can convert this if Windows port gets textInputController.hasSpellingMarkers().
Landed Bug 47885. It's time to make this happen!
I'm working on Bug 25539 and planning extend this test for that.
So landing this early would be helpful.
Comment 10 Kent Tamura 2010-10-22 01:14:25 PDT
Comment on attachment 71534 [details]
Patch

Looks good!
Comment 11 Hajime Morrita 2010-10-22 01:21:44 PDT
Committed r70295: <http://trac.webkit.org/changeset/70295>
Comment 12 WebKit Review Bot 2010-10-22 03:18:36 PDT
http://trac.webkit.org/changeset/70295 might have broken GTK Linux 64-bit Debug