[webkit-reviews] review granted: [Bug 74514] Need a way to produce leaner markup when pasting a fragment containing verbose markup : [Attachment 119299] Patch 2

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 74514: Need a way to produce leaner markup when pasting a fragment
containing verbose markup
https://bugs.webkit.org/show_bug.cgi?id=74514

Attachment 119299: Patch 2
https://bugs.webkit.org/attachment.cgi?id=119299&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119299&action=review


Okay, I think we can land as it, assuming you fix the address the index vs.
iterator issue below. Thanks for the clarifications! Please ping me if you
needed any help in rebaselining chromium tests.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:587
> +    for (Vector<Node*>::const_iterator it = nodesToRemove.begin(); it !=
nodesToRemove.end(); ++it)

Please use integral index instead of iterator as discussed on
https://lists.webkit.org/pipermail/webkit-dev/2011-October/018274.html.

> LayoutTests/editing/pasteboard/paste-and-sanitize.html:39
> +testPaste("div", "<div><b><i><span style=\"font-weight:
normal\"><b><i>Hello</i></b></span></i></b></div>", "<b><i>Hello</i></b>");

You may want to use single quotation to avoid having to cast ".

> LayoutTests/editing/pasteboard/paste-and-sanitize.html:47
> +	     "<i style=\"margin-top: 10px; margin-right: 10px; margin-bottom:
10px; margin-left: 10px; \"><b><i style=\"margin-top: 10px; margin-right: 10px;
margin-bottom: 10px; margin-left: 10px; \">hello</i></b></i>");

We really need to make cssText() use shorthand notations :(