Bug 43405 – Extract a function that serializes nodes from the range version of createMarkup

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

The range version of createMarkup currently does three things:
1. Disable "delete button" and adjust the starting node
2. Serialize nodes
3. Add style spans / divs to the serialized nodes to preserve styles.

As a result, createMarkup is large is a large function with plenty of local variables.
Because step 2 is straight forward operation, I propose to extract it as a function.

[Ryosuke Niwa]

Created attachment 63308 [details]
extracted serializeNodes

[Tony Chang]

Comment on attachment 63308 [details]
extracted serializeNodes

> Index: WebCore/editing/markup.cpp
> ===================================================================
> +static Node* serializeNodes(MarkupAccumulatorWrapper& accumulator, Node* startNode, Node* pastEnd, Vector<Node*>* nodes, const Range* range, EAnnotateForInterchange annotate, EAbsoluteURLs absoluteURLs)

Nit: Can we make |startNode| or |pastEnd| const?

[Ryosuke Niwa]

Thanks for the review ojan!

(In reply to comment #2)
> (From update of attachment 63308 [details])
> > Index: WebCore/editing/markup.cpp
> > ===================================================================
> > +static Node* serializeNodes(MarkupAccumulatorWrapper& accumulator, Node* startNode, Node* pastEnd, Vector<Node*>* nodes, const Range* range, EAnnotateForInterchange annotate, EAbsoluteURLs absoluteURLs)
> 
> Nit: Can we make |startNode| or |pastEnd| const?

Will fix and commit.

[Ryosuke Niwa]

(In reply to comment #3)
> > Nit: Can we make |startNode| or |pastEnd| const?
> 
> Will fix and commit.

As it turns out, this is not possible because startNode is assigned to n, which is passed to various functions which requires n to be non-const.  pastEnd is also assigned to next so we can't make pastEnd const either.

[Darin Adler]

Generally speaking I don't think we should ever use const Node* -- it doesn't make sense to say that a node in a tree is "const". What about the entire rest of the tree? Why is it OK for me to get a non-const pointer to my parent given a const pointer to myself. I think const has limited usefulness on DOM nodes and render tree nodes.

[Ryosuke Niwa]

Committed r64609: <http://trac.webkit.org/changeset/64609>