Bug 71386 - Use dumpAsText for printing layout test
Summary: Use dumpAsText for printing layout test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 71080
  Show dependency treegraph
 
Reported: 2011-11-02 12:37 PDT by Stephen Chenney
Modified: 2011-12-22 02:32 PST (History)
8 users (show)

See Also:


Attachments
patch (deleted)
2011-11-09 13:54 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for commit (deleted)
2011-11-09 15:22 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen Chenney 2011-11-02 12:37:23 PDT
The layout test printing/single-line-must-not-be-split-into-two-pages.html produces enormous expected image results, which appear to be causing problems. I suspect it is the PNGs themselves, as other tests using the same code path do not have similar issues.

There is no reason to use the image to validate this test - the text dump is adequate.
Comment 1 Kentaro Hara 2011-11-03 17:07:21 PDT
(In reply to comment #0)
> The layout test printing/single-line-must-not-be-split-into-two-pages.html produces enormous expected image results, which appear to be causing problems. I suspect it is the PNGs themselves, as other tests using the same code path do not have similar issues.
> 
> There is no reason to use the image to validate this test - the text dump is adequate.

An image is not necessary, but dumpAsText() is not enough. single-line-must-not-be-split-into-two-pages.html needs to check if no line is split across two pages, based on a DumpRenderTree result (i.e. single-line-must-not-be-split-into-two-pages-expected.txt). So what we need is the way we generate the DumpRenderTree result but do not generate the image even if --pixel option is specified for ./Tools/Scripts/new-run-webkit-tests. 

I could not find layoutTestControlloer.setXXXXXX() that works so, but do you know the way?
Comment 2 Stephen Chenney 2011-11-03 17:34:33 PDT
The behavior I see is that no image is generated by NRWT if dumpAsText is called on LayoutTestController inside the test, regardless of the --pixel-test argument. If only I could get webkit-patch to upload without trying to include a diff against a deleted file, all would be well.
Comment 3 Kentaro Hara 2011-11-03 18:37:37 PDT
(In reply to comment #2)
> The behavior I see is that no image is generated by NRWT if dumpAsText is called on LayoutTestController inside the test, regardless of the --pixel-test argument. If only I could get webkit-patch to upload without trying to include a diff against a deleted file, all would be well.

I think... no.

[1] If you specify dumpAsText(), then you can get not a DumpRenderTree result but a textContent result, like this:

WWWWW
WWWWW
WWWWW
WWWWW
...

With the textContent result, we cannot check if no line is split across two pages. We need the DumpRenderTree result.

[2] If you specify dumpAsText() and setPrinting() at the same time (although I think this is inherently bad), then you can get the textContent result on WebKit/Mac and the DumpRenderTree result on Chromium/Linux.


I am trying another approach to get the DumpRenderTree result without generating an image result.
Comment 4 Stephen Chenney 2011-11-07 08:52:33 PST
We should just change the test to be clear when it succeeds and when it fails. Right now I wouldn't know if a new result was good or bad. Let's not waste our effort trying to find a hack solution to a fundamental problem.
Comment 5 Kentaro Hara 2011-11-07 09:11:40 PST
(In reply to comment #4)
> We should just change the test to be clear when it succeeds and when it fails. Right now I wouldn't know if a new result was good or bad. Let's not waste our effort trying to find a hack solution to a fundamental problem.

The test checks if no line is split across two pages. The reason for one line being split into two pages is accumulation of rounding error in rendering calculation. Thus, a lot of pages were required to accumulate the rounding error much enough to reproduce the problem.

We might be able to reduce the number of pages by half or so, but it will not an essential solution.
Comment 6 Stephen Chenney 2011-11-07 12:39:30 PST
I've tried rolling back to the code before http://trac.webkit.org/changeset/95249, when the bug this test is associated with was fixed. However, the behavior is significantly different and I cannot repro it even with that older, supposedly broken code.

My idea was to reduce the line spacing for the test. If line-height:100% (i.e. no gaps between lines of text) then I would expect to see the line split problem happening on the first page, because there would be no whitespace to hide the error.

Also, it seems that even after one page the error in page height would manifest in an image comparison because the text would still end up on a different position in the page.

I will not change the test because I cannot repro the error to verify that the test show it. So I have pushed it onto haraken to try to verify that my ideas work. Or figure out some other way to reduce the test case.
Comment 7 Kentaro Hara 2011-11-09 13:54:03 PST
Created attachment 114359 [details]
patch
Comment 8 Tony Chang 2011-11-09 14:58:32 PST
Comment on attachment 114359 [details]
patch

I think this change is fine (less pixel results to maintain), but as someone not familiar with the test, it's not obvious how to tell if the test passes or not.  If there were some way to determine that (say by analyzing the text in script instead of dumping it a node), that would be better.
Comment 9 Kentaro Hara 2011-11-09 15:22:59 PST
Created attachment 114377 [details]
patch for commit

> it's not obvious how to tell if the test passes or not.  
> If there were some way to determine that (say by analyzing the text 
> in script instead of dumping it a node), that would be better.

I am afraid that it is difficult to show whether the test passes or not just by a text result. Instead, I added a comment about how to test it manually to single-line-must-not-be-split-into-two-pages.html.
Comment 10 WebKit Review Bot 2011-11-09 15:43:03 PST
Comment on attachment 114377 [details]
patch for commit

Clearing flags on attachment: 114377

Committed r99772: <http://trac.webkit.org/changeset/99772>