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

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied 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 119248: Patch
https://bugs.webkit.org/attachment.cgi?id=119248&action=review

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


> Source/WebCore/editing/ReplaceSelectionCommand.cpp:376
> +    , m_sanitizeFragment(options & SanitizeFragment)

Why can't we use this option always?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:572
> +	       if (currentNode && currentNode->parentNode() != rootNode &&
currentNode->firstChild() == currentNode->lastChild() &&
currentNode->hasTagName(divTag)) {
> +		   const NamedNodeMap* attributeMap =
currentNode->attributes();
> +		   if (!attributeMap || attributeMap->isEmpty())
> +		       nodesToRemove.append(currentNode);
> +	       }
> +
> +	       currentNode = currentNode->parentNode();
> +	       if (!currentNode->renderer() ||
!currentNode->renderer()->isRenderInline() ||
toRenderInline(currentNode->renderer())->alwaysCreateLineBoxes())
> +		   continue;

I think you want to reuse the logic in isStyleSpanOrSpanWithOnlyStyleAttribute.


> Source/WebCore/editing/ReplaceSelectionCommand.cpp:580
> +	       if (currentNode->renderStyle()->diff(startingStyle, context) ==
StyleDifferenceEqual)

You should be able to reuse the logic in EditingStyle::removeStyleAddedByNode.