Bug 133544 – Share mac/editing/spelling/editing-word-with-marker-2.html with other platforms

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

[Grzegorz Czajkowski]

Move mac/editing/spelling/editing-word-with-marker-2.html into common editing/spelling to be available for other platforms.

Additionally, verify spelling markers asynchronously as the sync path it's likely to be removed.

Add test case which tests spelling markers without any selection change.

[Grzegorz Czajkowski]

Created attachment 232541 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 232541 [details]
Patch

Rejecting attachment 232541 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 232541, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
Tests/platform/mac/editing/spelling/editing-word-with-marker-2-expected.txt'
patching file LayoutTests/platform/mac/editing/spelling/editing-word-with-marker-2.html
rm 'LayoutTests/platform/mac/editing/spelling/editing-word-with-marker-2.html'
patching file LayoutTests/platform/win/TestExpectations
Hunk #1 succeeded at 2001 (offset 67 lines).

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Darin Adler']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/6204263986364416

[Grzegorz Czajkowski]

Committed r169687: <http://trac.webkit.org/changeset/169687>

[Daniel Bates]

Comment on attachment 232541 [details]
Patch

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

Test is no longer doing what it saids it is doing after this change. Mind boggling that this change also skips or marks the test as failed on all platforms.

> LayoutTests/editing/spelling/editing-word-with-marker-2.html:63
> +var testCases = [

There is no longer a test for adding a space character after the marker, which is what the original test did do.

> LayoutTests/editing/spelling/editing-word-with-marker-2.html:74
> +        misspelledPosition: 7

This is not correct, it should be 9 not 7 because the code ALWAYS types a space character after evaluating updateCaretPosition.

[Darin Adler]

Should we roll it out?

[Darin Adler]

Should we revert this?

[Daniel Bates]

Comment on attachment 232541 [details]
Patch

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

>> LayoutTests/editing/spelling/editing-word-with-marker-2.html:74
>> +        misspelledPosition: 7
> 
> This is not correct, it should be 9 not 7 because the code ALWAYS types a space character after evaluating updateCaretPosition.

Never mind, I misread, there's a resetText() call between runs.

[Daniel Bates]

(In reply to Darin Adler from comment #5)
> Should we roll it out?

(In reply to Darin Adler from comment #6)
> Should we revert this?

I am going to try to fix up the test. Giving the benefit of doubt to the Grzegorz that it needed to be async because I am NOT planning to test this on Mac myself. I'll let the bots do it.

[Daniel Bates]

Comment on attachment 232541 [details]
Patch

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

> LayoutTests/editing/spelling/editing-word-with-marker-2.html:82
> +        return setTimeout(checkSpellingMarkerAfterAddingWhitespace(testCase), 0);

This doesn't actually call checkSpellingMarkerAfterAddingWhitespace on the next event loop iteration. It calls it immediately.

[Darin Adler]

Comment on attachment 232541 [details]
Patch

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

>> LayoutTests/editing/spelling/editing-word-with-marker-2.html:82
>> +        return setTimeout(checkSpellingMarkerAfterAddingWhitespace(testCase), 0);
> 
> This doesn't actually call checkSpellingMarkerAfterAddingWhitespace on the next event loop iteration. It calls it immediately.

And then sets a timer to do whatever the function returns, so I guess execute the string "undefined" or something like that.