splitTextAtStartIfNeed, splitTextAtEndIfNeed, splitTextElementAtStartIfNeed and splitTextElementAtEndIfNeed need to be cleaned up to fix the bug 39989.
Created attachment 62512 [details] clean up Split the cleanup from the patch for the bug 39989.
Attachment 62512 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/editing/ApplyStyleCommand.cpp:1538: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1545: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1548: Declaration has space between type name and * in Node *prevNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1550: Declaration has space between type name and * in Node *startNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1558: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1565: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1568: Declaration has space between type name and * in Node *prevNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1570: Declaration has space between type name and * in Node *startNode [whitespace/declaration] [3] Total errors found: 8 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 62513 [details] fixed needS
Attachment 62513 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/editing/ApplyStyleCommand.cpp:1538: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1545: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1548: Declaration has space between type name and * in Node *prevNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1550: Declaration has space between type name and * in Node *startNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1558: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1565: Declaration has space between type name and * in Text *text [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1568: Declaration has space between type name and * in Node *prevNode [whitespace/declaration] [3] WebCore/editing/ApplyStyleCommand.cpp:1570: Declaration has space between type name and * in Node *startNode [whitespace/declaration] [3] Total errors found: 8 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 62515 [details] fixed references and pointers (space before &/*)
Comment on attachment 62515 [details] fixed references and pointers (space before &/*) > - > -bool ApplyStyleCommand::splitTextElementAtEndIfNeeded(const Position &start, const Position &end) > + nit: Do not add whitespace at the end of line. > } > - > + > static bool areIdenticalElements(Node *first, Node *second) ditto.
In the ChangeLog "No new tests added." This should explain why no new tests were added. Was it because no user visible functionality should have changed?
(In reply to comment #6) > (From update of attachment 62515 [details]) > > > - > > -bool ApplyStyleCommand::splitTextElementAtEndIfNeeded(const Position &start, const Position &end) > > + > > nit: Do not add whitespace at the end of line. > > > } > > - > > + > > static bool areIdenticalElements(Node *first, Node *second) > > ditto. Thanks for the review & style check. I didn't catch that. (In reply to comment #7) > In the ChangeLog "No new tests added." > > This should explain why no new tests were added. Was it because no user visible functionality should have changed? I thought it's quite normal to omit the reason or mentioning of test at all when the bug itself says it's a clean up. But I'll change it to "No new tests added since this is a clean up" per your request.
Landed as http://trac.webkit.org/changeset/64028.