[webkit-reviews] review denied: [Bug 62464] [Qt] QWebElement::encloseWith doesn't work at all : [Attachment 106858] patch for review.

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Zeno Albisser
<zeno.albisser at nokia.com>'s request for review:
Bug 62464: [Qt] QWebElement::encloseWith doesn't work at all
https://bugs.webkit.org/show_bug.cgi?id=62464

Attachment 106858: patch for review.
https://bugs.webkit.org/attachment.cgi?id=106858&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106858&action=review


Looks generally great, apart from some minor coding style problems.
Please refer to http://www.webkit.org/coding/coding-style.html for the inside
scoop :)
You can use the Tools/Scripts/check-webkit-style script to verify correct style
before uploading patches.

> Source/WebKit/qt/Api/qwebelement.cpp:1122
> +    RefPtr<DocumentFragment> fragment = 
Range::createDocumentFragmentForElement(markup, toHTMLElement(parent));

Coding style, two spaces after =.

> Source/WebKit/qt/Api/qwebelement.cpp:1172
> +    RefPtr<DocumentFragment> fragment = 
Range::createDocumentFragmentForElement(markup, toHTMLElement(parent));

Ditto.

> Source/WebKit/qt/Api/qwebelement.cpp:1393
> +    RefPtr<DocumentFragment> fragment = 
Range::createDocumentFragmentForElement(markup, toHTMLElement(parent));

Ditto.

> Source/WebKit/qt/tests/qwebelement/tst_qwebelement.cpp:650
> +    // check behavior for a tag that IE forbids inserting HTML

Coding style, comments should be written like normal sentences (capitalized and
ending with a period.)

> Source/WebKit/qt/tests/qwebelement/tst_qwebelement.cpp:655
> +    QWebElement img = body.findFirst("img");
> +    img.appendInside("<p id=\"fail1\"></p>");

QVERIFY(!img.isNull()); wouldn't hurt here.

> Source/WebKit/qt/tests/qwebelement/tst_qwebelement.cpp:904
> +    // check behavior for a tag that IE forbids inserting HTML

Coding style, comments should be written like normal sentences (capitalized and
ending with a period.)

> Source/WebKit/qt/tests/qwebelement/tst_qwebelement.cpp:909
> +    QWebElement img = body.findFirst("img");
> +    img.encloseWith("<p id=\"success\"></p>");

QVERIFY(!img.isNull()); wouldn't hurt here.