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.
Created attachment 232541 [details] Patch
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
Committed r169687: <http://trac.webkit.org/changeset/169687>
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.
Should we roll it out?
Should we revert this?
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.
(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.
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.
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.