Bug 128526 – Use asynchronous spellchecking in spelling-hasspellingmarker.js

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Grzegorz Czajkowski]

Refactoring spelling-hasspellingmarker.js to use asynchronous spellchecking.

[Grzegorz Czajkowski]

Created attachment 223695 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 223695 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223695&action=review

> LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:40
> +    if (window.internals)
> +        shouldBecomeEqual("internals.hasSpellingMarker(0, 2)", isMisspelled ? "true" : "false", done);
> +    else
> +        done();

Why are we calling done in the case window.internals is not defined?

> LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:47
> +var tests = [ function() { verifySpellingMarkers("<textarea></textarea>", true); },
> +              function() { verifySpellingMarkers("<input type='text'></input>", true); },
> +              function() { verifySpellingMarkers("<div contenteditable='true'></div>", true); },
> +              function() { verifySpellingMarkers("<div contentEditable='true' spellcheck='false'></div>", false); }
> +            ];

Wrong indentations. The subsequent line should be indented by exactly 4 spaces.
It also seems that these could be simply markup & boolean objects as in:
{containerMarkup: "<textarea></textarea>", isMisspelled: true}

> LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:53
> +        return window.setTimeout(next, 0);

We don't need "window."

[Grzegorz Czajkowski]

(In reply to comment #2)
> (From update of attachment 223695 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223695&action=review
> 
> > LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:40
> > +    if (window.internals)
> > +        shouldBecomeEqual("internals.hasSpellingMarker(0, 2)", isMisspelled ? "true" : "false", done);
> > +    else
> > +        done();
> 
> Why are we calling done in the case window.internals is not defined?

I added it here to just satisfy non-DumpRenderTree users as done() invokes next test case that is defined in 'var tests = [ function(), function() ... ]'. Without it, Browser wouldn't create desired element for given markup. Maybe it could be better to first create such elements and then call verifySpellingMarkers() with early return when internals are not available?

> 
> > LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:47
> > +var tests = [ function() { verifySpellingMarkers("<textarea></textarea>", true); },
> > +              function() { verifySpellingMarkers("<input type='text'></input>", true); },
> > +              function() { verifySpellingMarkers("<div contenteditable='true'></div>", true); },
> > +              function() { verifySpellingMarkers("<div contentEditable='true' spellcheck='false'></div>", false); }
> > +            ];
> 
> Wrong indentations. The subsequent line should be indented by exactly 4 spaces.

Right. I'll fix them.

> It also seems that these could be simply markup & boolean objects as in:
> {containerMarkup: "<textarea></textarea>", isMisspelled: true}
> 
> > LayoutTests/editing/spelling/script-tests/spelling-hasspellingmarker.js:53
> > +        return window.setTimeout(next, 0);
> 
> We don't need "window."

Ok.

[Grzegorz Czajkowski]

Created attachment 223842 [details]
took into account Ryosuke's comments

[Grzegorz Czajkowski]

Committed r164100: <http://trac.webkit.org/changeset/164100>