Change TextIterator to use StringView, preparing to wean it from deprecatedCharacters
Created attachment 223458 [details] Patch
Comment on attachment 223458 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=223458&action=review > Source/WebCore/platform/text/mac/TextBoundaries.mm:219 > + RetainPtr<CFStringRef> uniString = text.createCFStringWithoutCopying(); > + RetainPtr<CFStringRef> foundWord = text.substring(*start, *end - *start).createCFStringWithoutCopying(); These can both be auto.
Comment on attachment 223458 [details] Patch Attachment 223458 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5326948951654400 New failing tests: editing/pasteboard/bad-placeholder.html editing/execCommand/createLink.html editing/execCommand/button.html editing/pasteboard/8145-2.html editing/execCommand/findString-2.html editing/inserting/insert-paragraph-03.html editing/pasteboard/5006779.html editing/selection/4932260-2.html editing/inserting/4960120-2.html editing/execCommand/6355786.html editing/input/password-echo-passnode2.html editing/execCommand/findString.html editing/input/password-echo-passnode.html editing/input/emacs-ctrl-o.html editing/execCommand/hilitecolor.html editing/inserting/insert-before-link-1.html editing/pasteboard/4806874.html editing/input/password-echo-passnode3.html editing/deleting/delete-cell-contents.html editing/selection/4983858.html editing/execCommand/findString-3.html editing/selection/4932260-3.html editing/execCommand/indent-pre.html editing/execCommand/findString-diacriticals.html editing/deleting/delete-by-word-001.html editing/deleting/delete-by-word-002.html editing/inserting/insert-paragraph-04.html editing/deleting/5300379.html editing/pasteboard/4944770-1.html editing/pasteboard/4242293-1.html
Created attachment 223464 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 223458 [details] Patch Attachment 223458 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5051042400043008 New failing tests: editing/pasteboard/bad-placeholder.html editing/execCommand/createLink.html editing/execCommand/button.html editing/pasteboard/8145-2.html editing/execCommand/findString-2.html editing/inserting/insert-paragraph-03.html editing/pasteboard/5006779.html editing/selection/4932260-2.html editing/inserting/4960120-2.html editing/execCommand/6355786.html editing/input/password-echo-passnode2.html editing/execCommand/findString.html editing/input/password-echo-passnode.html editing/input/emacs-ctrl-o.html editing/execCommand/hilitecolor.html editing/inserting/insert-before-link-1.html editing/pasteboard/4806874.html editing/input/password-echo-passnode3.html editing/deleting/delete-cell-contents.html editing/selection/4983858.html editing/execCommand/findString-3.html editing/selection/4932260-3.html editing/execCommand/indent-pre.html editing/execCommand/findString-diacriticals.html editing/deleting/delete-by-word-001.html editing/deleting/delete-by-word-002.html editing/inserting/insert-paragraph-04.html editing/deleting/5300379.html editing/pasteboard/4944770-1.html editing/pasteboard/4242293-1.html
Created attachment 223468 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 223458 [details] Patch Attachment 223458 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5275092456046592 New failing tests: editing/selection/5195166-1.html editing/execCommand/createLink.html editing/execCommand/button.html editing/execCommand/find-after-replace.html editing/pasteboard/4944770-1.html editing/execCommand/findString-2.html editing/inserting/insert-paragraph-03.html editing/selection/4932260-2.html editing/inserting/4960120-2.html editing/execCommand/6355786.html editing/deleting/delete-by-word-002.html editing/execCommand/findString.html editing/input/password-echo-passnode.html editing/input/emacs-ctrl-o.html editing/execCommand/hilitecolor.html editing/inserting/insert-before-link-1.html editing/pasteboard/4806874.html editing/input/password-echo-passnode3.html editing/deleting/delete-cell-contents.html editing/selection/4983858.html editing/execCommand/findString-3.html editing/selection/4932260-3.html editing/execCommand/indent-pre.html editing/execCommand/findString-diacriticals.html editing/deleting/delete-by-word-001.html editing/inserting/insert-paragraph-04.html editing/deleting/5300379.html editing/input/password-echo-passnode2.html editing/pasteboard/4242293-1.html editing/pasteboard/8145-2.html
Created attachment 223499 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 223564 [details] Patch
Created attachment 223565 [details] Patch
Created attachment 223566 [details] Patch
Committed r163712: <http://trac.webkit.org/changeset/163712>
Comment on attachment 223566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=223566&action=review While reviewing this patch because of bug #133047 I've found a couple of issues, although they didn't seem to cause the regression. > Source/WebCore/editing/TextCheckingHelper.cpp:80 > - int wordStart = textBreakCurrent(iterator); > - while (0 <= wordStart) { > + for (int wordStart = textBreakCurrent(iterator); wordStart > 0; ) { The while loop condition was wordStart >= 0, shouldn't the for loop one be wordStart >= 0 too? > Source/WebCore/editing/TextCheckingHelper.cpp:258 > - if (!(len == 1 && chars[0] == ' ')) { > - > + if (textLength == 1 && text[0] == ' ') { This was !() before, I guess we don't want to enter the if when the text is just one space.
(In reply to comment #13) > > Source/WebCore/editing/TextCheckingHelper.cpp:80 > > - int wordStart = textBreakCurrent(iterator); > > - while (0 <= wordStart) { > > + for (int wordStart = textBreakCurrent(iterator); wordStart > 0; ) { > > The while loop condition was wordStart >= 0, shouldn't the for loop one be wordStart >= 0 too? > > > Source/WebCore/editing/TextCheckingHelper.cpp:258 > > - if (!(len == 1 && chars[0] == ' ')) { > > - > > + if (textLength == 1 && text[0] == ' ') { > > This was !() before, I guess we don't want to enter the if when the text is just one space. Yes, you are right about both. I guess we don’t have any test coverage for either of these code paths. Ideally we’d make a regression test for this, but I think we should submit a patch right away to fix both even without a test. Would you be willing to do that, or would you like me to do it (or get someone else to do it)?
(In reply to comment #14) > (In reply to comment #13) > > > Source/WebCore/editing/TextCheckingHelper.cpp:80 > > > - int wordStart = textBreakCurrent(iterator); > > > - while (0 <= wordStart) { > > > + for (int wordStart = textBreakCurrent(iterator); wordStart > 0; ) { > > > > The while loop condition was wordStart >= 0, shouldn't the for loop one be wordStart >= 0 too? > > > > > Source/WebCore/editing/TextCheckingHelper.cpp:258 > > > - if (!(len == 1 && chars[0] == ' ')) { > > > - > > > + if (textLength == 1 && text[0] == ' ') { > > > > This was !() before, I guess we don't want to enter the if when the text is just one space. > > Yes, you are right about both. > > I guess we don’t have any test coverage for either of these code paths. Ideally we’d make a regression test for this, but I think we should submit a patch right away to fix both even without a test. Would you be willing to do that, or would you like me to do it (or get someone else to do it)? I was indeed wrong, those changes (actually the second one) fix the regression in the GTK+ port, but I forgot to enable the spell checker when I tested it :-P I'll write the patch.