Verify spellcheck attribute asynchronously

Verify spellcheck attribute asynchronously

Refactoring spelling tests that verify spellcheck attribute to use
asynchronous text checking.

Additionally, add the elements that are being tested to to the tree.
It gives a possibility for non-DumpRenderTree users to see misspellings.

Fix the description message according to the test behavior.

This change is based on WebKit changeset: http://trac.webkit.org/changeset/163432

BUG=295722
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168998

[tony, 2014-03-10 17:07:56]

LGTM, some style nits.

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
File LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js
(right):

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js:20:
document.getSelection().setPosition(childText, 1);  // [^]
I don't understand the comment. Either write out a sentence or remove it.

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js:37:
function() { verifyChildSpellingMarker("<div contentEditable>Foo <span
spellcheck='false' id='child'>[]</span> Baz</div>", false); },
This is fine, but it might be easier to refactor out the closure and
verifyChildSpellingMarker into the done function.

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
File LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js
(right):

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js:59:
function() { testFor("SPAN", true, true); }, // true -> true
I'm not sure there's a lot of value to these comments.  This could also be
["SPAN", true, true] for each test case and the closure created in done().

[groby-ooo-7-16, 2014-03-10 20:14:04]

LGTM, with a tiny improvement wish. (Feel free to ignore if you think it's not
making things cleaner - it's on the border, I think)

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
File LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js
(right):

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js:59:
function() { testFor("SPAN", true, true); }, // true -> true
On 2014/03/10 17:07:56, tony wrote:
> I'm not sure there's a lot of value to these comments.  This could also be
> ["SPAN", true, true] for each test case and the closure created in done().

And while you're simplifying: The testFor function is always called for the same
6 initial/expectation combos - might want to iterate through that in done(),
too.

[grzegorz, 2014-03-11 13:24:32]

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
File LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js
(right):

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js:20:
document.getSelection().setPosition(childText, 1);  // [^]
On 2014/03/10 17:07:56, tony wrote:
> I don't understand the comment. Either write out a sentence or remove it.

My intention here was to show caret position at the child node as its content is
'[]'. However, I removed it.

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-at-child.js:37:
function() { verifyChildSpellingMarker("<div contentEditable>Foo <span
spellcheck='false' id='child'>[]</span> Baz</div>", false); },
On 2014/03/10 17:07:56, tony wrote:
> This is fine, but it might be easier to refactor out the closure and
> verifyChildSpellingMarker into the done function.

Thanks. I am not sure if I understood it correctly so please have a look at
patch #2. However, we could move forward and apply those changes in all tests
that use done() function. As far as I know, it's duplicated in many
spellchecking tests. We could define something like TestCaseController which
would take two parameters:
1) a testing function
2) a table of test case.
and then we could put it for example in spelling/resources/util.js which is
commonly used.

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
File LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js
(right):

https://codereview.chromium.org/190663014/diff/1/LayoutTests/editing/spelling...
LayoutTests/editing/spelling/script-tests/spelling-attribute-change.js:59:
function() { testFor("SPAN", true, true); }, // true -> true
On 2014/03/10 20:14:09, groby wrote:
> On 2014/03/10 17:07:56, tony wrote:
> > I'm not sure there's a lot of value to these comments.  This could also be
> > ["SPAN", true, true] for each test case and the closure created in done().
> 
> And while you're simplifying: The testFor function is always called for the
same
> 6 initial/expectation combos - might want to iterate through that in done(),
> too.

Thanks. Please have a look at patch #2 if I properly applied those suggestions.

[tony, 2014-03-11 17:02:42]

Yes, that's what I had in mind.  Thanks! LGTM.

[groby-ooo-7-16, 2014-03-11 17:32:00]

Awesome, thank you - LGTM

[grzegorz, 2014-03-12 07:20:57]

The CQ bit was checked by g.czajkowski@samsung.com

[commit-bot: I haz the power, 2014-03-12 07:21:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/g.czajkowski@samsung.com/190663014/20001

[commit-bot: I haz the power, 2014-03-12 08:22:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-12 08:22:04]

Try jobs failed on following builders: linux_blink_rel

[grzegorz, 2014-03-12 08:26:36]

The CQ bit was checked by g.czajkowski@samsung.com

[commit-bot: I haz the power, 2014-03-12 08:26:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/g.czajkowski@samsung.com/190663014/20001

[commit-bot: I haz the power, 2014-03-12 09:23:17]

Change committed as 168998