[webkit-qt] printing unit tests

[Simon Hausmann]

On Friday, April 13, 2012 04:38:16 PM ext Milian Wolff wrote:
> On Wednesday 11 April 2012 20:05:39 Milian Wolff wrote:
> > Hey all,
> > 
> > I'm interested in improving the printing support of the QtWebKit port.
> > 
> > I notice that a lot of unit tests are skipped and that apparently
> > isPrinting() is not implemented/handled at all yet.
> > 
> > For quite some time now I'm trying to figure out what exactly would be
> > required to get this done but it seems to be far from easy... Is there any
> > documentation besides the chromium spagetthi test code on how the output
> > should look like or when to output what (image, text, both, ...)?
> > 
> > As far as I can see, I will probably have to handle the isPrinting() mode
> > in DumpRenderTreeQt.cpp ~ ::dump() in the if(m_dumpPixels) branch.
> > Currently, that only paints the viewportSize() to an image. If I'm not
> > mistaken, one needs to create a copy or QWebFrame::print, similar to
> > chromiums'
> > WebViewHost::paintPagesWithBoundaries. But what are the requirements on
> > the
> > output? Is there *any* documentation on what the tests should output?
> 
> I've done this now, see the attached patch. It kind-of-works, e.g. for
> media- queries-print.html: http://imagebin.org/207870

Like Andras said, patches are best kept in Bugzilla where we have a beautiful 
tool for reviewing them - line by line :)
 
> But for setPrint.html I see horrible render bug which I do not the reason
> to:
> 
> http://imagebin.org/207871
> 
> Note how the right border of the box is not properly drawn. Furthermore, the
> box is one pixel too high, or well - the bottom border is painted one pixel
> too far below. Compare to e.g.
> LayoutTests/platform/mac/printing/setPrinting- expected.png to see how it
> should look like.
> 
> So... has anyone an idea of what could be going on here?

I don't know off-hand, but these one-pixel-off differences are exactly one of the 
reasons why pixel tests are difficult to maintain. Different software rasterizers 
for example have different interpretations about pixel alignment, which may or 
may not be a possible reason for this.

I think you're on the right track with the patch (passing the height) and I 
_do_ think it is worthwhile to add pixel test support to DRT for isPrinting(), 
but for this particular bug I would also suggest to resort to the approach of 
trying to pass the existing *-expected.txt tests. If the existing tests don't 
cover this particular issue (missing height), then we need a different way to 
compare the symptoms.


Simon