[webkit-reviews] review granted: [Bug 19795] execCommand FormatBlock creates lots of blockquotes : [Attachment 70586] fixed moveParagraphWithClones for fast/html/nav-element.html

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 19795: execCommand FormatBlock creates lots of blockquotes
https://bugs.webkit.org/show_bug.cgi?id=19795

Attachment 70586: fixed moveParagraphWithClones for fast/html/nav-element.html
https://bugs.webkit.org/attachment.cgi?id=70586&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=70586&action=review

> WebCore/editing/EditorCommand.cpp:445
> +    if
(!FormatBlockCommand::isElementToApplyInFormatBlockCommand(qualifiedTagName))
> +	   return false;
> +
>      applyCommand(FormatBlockCommand::create(frame->document(),
qualifiedTagName));

If I was doing this, I would have put this logic into
FormatBlockCommand::create and had it return 0 if it’s an element it should not
apply to. That seems easier to use than a separate function you have to know to
call.

> WebCore/editing/FormatBlockCommand.cpp:78
> +// FIXME: We should consider mering this function with
isElementForFormatBlockCommand in Editor.cpp

Typo: "mering".

> WebCore/editing/FormatBlockCommand.h:41
> +    static bool isElementToApplyInFormatBlockCommand(const QualifiedName&
tagName);

I don’t understand the phrase “element to apply in command”. What does that
mean?