[webkit-reviews] review denied: [Bug 28781] Add QWebFrame::renderElement to API : [Attachment 40143] Updated QWebElement::render implementation with API autotest

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 6 03:44:58 PDT 2009


Ariya Hidayat <ariya.hidayat at trolltech.com> has denied Viatcheslav Ostapenko
<ostapenko.viatcheslav at nokia.com>'s request for review:
Bug 28781: Add QWebFrame::renderElement to API
https://bugs.webkit.org/show_bug.cgi?id=28781

Attachment 40143: Updated QWebElement::render implementation with API autotest
https://bugs.webkit.org/attachment.cgi?id=40143&action=review

------- Additional Comments from Ariya Hidayat <ariya.hidayat at trolltech.com>

> +/*!
> +  Render the element into \a painter .
> +*/

Needs \since tag.

> +<file>image.png</file>

Can we have other name that resembles the content of the test image?
image.png sounds too generic, we might want to add images for future tests.

> +    QCOMPARE(image1 == testImage, true);

I doubt this is going to work properly, we need pixel-by-pixel comparison (see
autotest for QImage in Qt source tree).
Since this is used in few places, it's a good idea to refactor to a function,
e.g.
static bool isImageEqual(const QImage &img1, const QImage &img2);


Also: see Simon's comment on Frame::nodeImage().


More information about the webkit-reviews mailing list