Followup fix for Bug 5241. See https://bugs.webkit.org/show_bug.cgi?id=5241#c34 for detail.
Created attachment 79951 [details] fixes the bug
Comment on attachment 79951 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=79951&action=review > Source/WebCore/editing/htmlediting.cpp:359 > + Vector<UChar> rebalanacedString; s/rebalanacedString/rebalancedString Also, why use a Vector<UChar> instead of String like the old code did? > Source/WebCore/editing/htmlediting.h:239 > // Miscellaneous functions on String > - > +inline bool isWhitespace(UChar c) > +{ > + return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t'; > +} Nit: technically, this is not a function on String, os the comment is not correct.
Comment on attachment 79951 [details] fixes the bug Whoops, meant to r-.
Comment on attachment 79951 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=79951&action=review >> Source/WebCore/editing/htmlediting.cpp:359 >> + Vector<UChar> rebalanacedString; > > s/rebalanacedString/rebalancedString > > Also, why use a Vector<UChar> instead of String like the old code did? As far I checked, we can't modify each character in String directly (because multiple String instances may share single StringImpl). i.e. String only has UChar operator[](unsigned index) const. >> Source/WebCore/editing/htmlediting.h:239 >> +} > > Nit: technically, this is not a function on String, os the comment is not correct. Maybe I should change the comment to "Miscellaneous functions on Text"?
Created attachment 80006 [details] fixed typos & comment per ojan's comment
Attachment 80006 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7585300
Attachment 80006 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7645057
Created attachment 80007 [details] fixed one more typo
Attachment 80006 [details] did not build on qt: Build output: http://queues.webkit.org/results/7542325
Attachment 80006 [details] did not build on win: Build output: http://queues.webkit.org/results/7621313
Comment on attachment 79951 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=79951&action=review >>> Source/WebCore/editing/htmlediting.cpp:359 >>> + Vector<UChar> rebalanacedString; >> >> s/rebalanacedString/rebalancedString >> >> Also, why use a Vector<UChar> instead of String like the old code did? > > As far I checked, we can't modify each character in String directly (because multiple String instances may share single StringImpl). i.e. String only has UChar operator[](unsigned index) const. I don't understand. Isn't that what the old code was doing with the "replace" calls?
Comment on attachment 80007 [details] fixed one more typo Ryosuke informed me that replace copies the string. So the new code is certainly more efficient.
(In reply to comment #12) > (From update of attachment 80007 [details]) > Ryosuke informed me that replace copies the string. So the new code is certainly more efficient. Note: new function creates copy regardless of whether there are whitespace or not while the old implementation doesn't make a copy when there are no whitespace. However, in practice, almost all text contains whitespace so this shouldn't be a problem. We can add that optimization later if we're so inclined.
Committed r76565: <http://trac.webkit.org/changeset/76565>