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.
Created attachment 63308 [details] extracted serializeNodes
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?
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.
(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.
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.
Committed r64609: <http://trac.webkit.org/changeset/64609>