Bug 42937 – splitTextAt*IfNeed and splitTextElementAt*IfNeed need to be cleaned up

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

[Ryosuke Niwa]

splitTextAtStartIfNeed, splitTextAtEndIfNeed, splitTextElementAtStartIfNeed and splitTextElementAtEndIfNeed need to be cleaned up to fix the bug 39989.

[Ryosuke Niwa]

Created attachment 62512 [details]
clean up

Split the cleanup from the patch for the bug 39989.

[WebKit Review Bot]

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.

[Ryosuke Niwa]

Created attachment 62513 [details]
fixed needS

[WebKit Review Bot]

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.

[Ryosuke Niwa]

Created attachment 62515 [details]
fixed references and pointers (space before &/*)

[Kent Tamura]

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.

[David Levin]

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?

[Ryosuke Niwa]

(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.

[Ryosuke Niwa]

Landed as http://trac.webkit.org/changeset/64028.