[Webkit-unassigned] [Bug 28781] Add QWebFrame::renderElement to API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28781





--- Comment #20 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2009-10-09 15:44:50 PDT ---
(In reply to comment #18)
> (From update of attachment 40143 [details])
> 
> > +/*!
> > +  Render the element into \a painter .
> > +*/
> 
> Needs \since tag.

I assume it should be 
\since 4.6
But in this case the whole QWebElement class has \since 4.6 .
Should I put the same tag also for render method?

> > +<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.

I was sure it will cause questions.
This image is just a copy from the qwebframe test. 1st I wanted to make a
reference to the qwebframe image, but noticed that all test cases do not share
any resources.
I would suggest to move all test resource files (images, htmls and etc.) into
common resource directory, but could it be done after this patch?

> > +    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);

What do you mean "work properly"?
I've looked at the QImage "==" operator and I think it does exactly what I
need.
IMHO, in QImage autotest the QImage class functionality is under test and
that's why test cannot rely on it.
In QWebElement test QImage functionality is assumed to be tested, as I think ;)

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

Done!

Thanks a lot,
  Sl

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.