307367 - [BC] print preview doesn't match print for thin borders on windows

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Neil Katin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6


Table borders look different between "print preview" and actual
printed page.

Border-collapse seems to play a major role in the bug.

If border-collapse is set to "collapse" then thin borders disappear.

If border-collapse is set to "separate" (the default) then
thick borders become thin, and thin borders are OK.

Once again: this is *only* in the printed copy; on screen and print
preview look OK.

Test case:

http://www.askneil.com/mozilla/testcase.html

The main web page view and the "print preview" view look the
same, and correct.  However when you actually print things look
different: see this page for an example:

http://www.askneil.com/mozilla/results.pdf

IE gets it right:

http://www.askneil.com/mozilla/results-ie.pdf

This is a windows-only bug; it worked fine on linux.
I was running windows XP SP 2; I didn't test other versions.

Reproducible: Always

Steps to Reproduce:
1. view http://www.askneil.com/mozilla/testcase.html
2. see print preview; should look the same
3. print the page (either to paper or to a pdf printer; I used www.cutepdf.com);
note how borders either disapper or change thickness.





Related bugs (but don't quite seem to be the same):
https://bugzilla.mozilla.org/show_bug.cgi?id=209664
https://bugzilla.mozilla.org/show_bug.cgi?id=138484
https://bugzilla.mozilla.org/show_bug.cgi?id=295289
https://bugzilla.mozilla.org/show_bug.cgi?id=142998
https://bugzilla.mozilla.org/show_bug.cgi?id=163445

Comment 1

[imacat]

Attachment: Thin Table Border Printing Test

The thin table border printing test file.

Comment 2

[imacat]

Attachment: Thin Table Border Printing Test Result, with Firefox 1.0.7

The print result of thin table border printing test, with Firefox 1.0.7.  The actual print result is the exactly same (on HP LaserJet 2200d and FinePrint 5.42).

Comment 3

[imacat]

Attachment: Thin Table Border Printing Test Result, with MSIE 6

The thin border printing test result with MSIE 6.0.2900.2180 on Windows XP SP2.

Comment 4

[imacat]

Ahh...  Nobody saw this report?  I've experiencing the same problem with my Firefox 1.0.7.  The table borders are not actually printed when its names are assigned by names (thin, medium, thick, see http://www.w3.org/TR/CSS21/box.html#border-width-properties) in collapse table border mode.  I worked around it by assigning borders by pt in the print stylesheet @media print {}, but this should not be necessary.  I have attached 3 files that may demostrate the problem, one HTML source, one PDF and one PDF created with MSIE.  Please someone check and fix this issue.  Thank you.

Comment 5

[Wayne Mery (:wsmwk)]

WFM trunk build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050917 SeaMonkey/1.1a

So it would appear to have been fixed for next release. Please try newer builds from http://www.mozillazine.org/ either nightly or 1.5 release candidate and report back. And also close the bug if one of those works for you. THanks.

Comment 6

[Neil Katin]

Attachment: Test results for firefox 1.5 rc2

I tried running the test against firefox 1.5 rc2.  Curiously enough now the hard-copy print looks different from printing to cute-pdf (to capture the
printer output).

My hard copy has no lines at all around the "test-fail" table in the test
case.  Cute-pdf output seems to have a thick line above "test-fail" and
no lines on the side (which is what it did last time).  I'm using the
test cases in comment #1.

I tried the test cases in the attachment to the bug.  They too look
different between the print-preview and captured pdf output.  In
particular "testing-table-2" is missing the thick border on the
outside.

I'm going to retry the bug using a nightly instead of 1.5RC2 and see
if it is different.


The "test-better" has thin lines on all sides (which is what it
always has done: not correct, but at least no worse).

Comment 7

[Neil Katin]

I just tried the nightly from 17 Nov 2005 (labelled
version 1.6a1).

It fails identially to 1.5rc2.

 

Comment 8

[imacat]

Well, since Neil had verified that the problem still exists, I'll not test right now.  Please say if you do need me to test it, too, or a newer CVS release is requesting tests.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Need minimal testcase (smallest HTML that still shows the problem).

Comment 10

[imacat]

(In reply to comment #9)
> Need minimal testcase (smallest HTML that still shows the problem).
> 

    I believe I have attached one.  See attached "Thin Table Border Printing Test".

    By the way, the problem is exactly the same under Firefox 1.5.  So I do not submit a new testing result.  Please tell me if you need any more information.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> See attached "Thin Table Border Printing Test".

So all of the tables there are needed to see the problem?  Again, this needs a _minimal_ testcase (one in which removing a single character from the HTML makes the buggy behavior disappear).

Comment 12

[imacat]

(In reply to comment #11)
> > See attached "Thin Table Border Printing Test".
> 
> So all of the tables there are needed to see the problem?  Again, this needs a
> _minimal_ testcase (one in which removing a single character from the HTML
> makes the buggy behavior disappear).

    YES.  I think ALL THE TABLES ARE NEEDED TO SEE THE PROBLEM.  Could you be so kind to view the "Thin Table Border Printing Test Result, with Firefox 1.0.7"?  Could you please be so kind to actually print that on an A4 paper with a printer and compare it with the print preview?  Table 1 and 2 should be printed "almost the same", and so is table 3 and 4.  Please specify clearly what is not needed here.

    There is no "removing a single character from the HTML makes the buggy behavior disappear" version.  At least to my limited talent.  I'm terribly sorry.

    I may not be famous, but I'm not new to bug submission.  I know what you are talking about.  Please don't treat me like I'm an idiot user.  It's quite insulting.

Comment 13

[Neil Katin]

Attachment: simpler test case (simple307367.html)

I attached a simplified version of my original test case: one table, one CSS
block.  Let me know if that is good enough to let you make progress.

Comment 14

[Neil Katin]

Attachment: pdf capture of printed output from simple307367.html

This is a capture of the current, incorrect 1.5 output.
This was captured on windows xp using a CutePDF virtual
printer:

  http://www.cutepdf.com/Products/CutePDF/writer.asp

This is a free download, and very useful to avoid having to
print a page for every test.

Comment 15

[Boris Zbarsky [:bzbarsky]]

imacat -- I wasn't saying YOU have to write a minimal testcase.  If you can, that's great.  If not, that's ok.  In case you missed it, I cced someone who often creates excellent minimal testcases when I commented.

Neil -- thanks a ton for the much smaller testcase.  I'm going to try to look at that in a debug build when I get back in a week; hopefully I can get somewhere.

Comment 16

[Bernd]

The computed border for the table in twips during printing is: 
top:    6
left:   1
right:  1
bottom: 4

during print preview:
top:    75
left:   15
right:  15
bottom: 45

during normal display:
top:    75
left:   15
right:  15
bottom: 45

we then round down towards pixel inside the BC computing and the border vanishes as twipstopixel is 0.0666667 = 1/15.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Er... twipstopixel should be 1/15 for screen (and that's what print preview and normal display are clearly using, since wide/medium/thin borders are 5/3/1 in pixels).  But for printing, are we using different values of t2p or something?  The values for top/left/bottom suggest a t2p of close to 1, not 1/15...

Is the problem that ScaledPixelsToTwips() is returning one thing and PixelsToTwips() returning something else and we're using one when we should use the other?

Comment 18

[Bernd]

changing the border width from thick and thin to explicit pixel sizes also fixes the printout.

Comment 19

[Bernd]

http://lxr.mozilla.org/seamonkey/source/layout/style/nsRuleNode.cpp#202
vs
http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresContext.cpp#732

3,2, 1, meins

Comment 20

[Bernd]

/layout/tables/nsTableFrame.cpp, line 3514 -- p2t = presContext->PixelsToTwips();
/layout/tables/nsTableFrame.cpp, line 4140 -- p2t = GetPresContext()->PixelsToTwips();
/layout/tables/nsTableRowGroupFrame.cpp, line 525 -- p2t = aPresContext->PixelsToTwips();
/layout/tables/nsTableCellFrame.cpp, line 310 -- PRInt16 t2p = (PRInt16) aPresContext->PixelsToTwips();
/layout/tables/nsTableCellFrame.cpp, line 1280 -- float p2t = GetPresContext()->PixelsToTwips();
will go too

Comment 21

[Bernd]

given the experience in this bug http://landfill.mozilla.org/mxr-test/seamonkey/search?string=-%3EPixelsToTwip

nearly every use is questionable once printing is involved....

Comment 22

[Bernd]

see bug 312214 for a random frame from the list

Comment 23

[Wayne Mery (:wsmwk)]

Given how long bc problems have existed, it's hard to imagine this wouldn't depend some other bug. Bug 138484 isn't a dup or similar?
Do any print related bc bugs depend on anything noted in Bug 203686?

recinding my comment 5, fails using attachment 207026 [details] - Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

[note bug 209664 in Neil's comment 0 appears to contain a typo]

Comment 24

[Bernd]

Attachment: patch

This removes any reference inside the table code to PixelstoTwips and ensures that always the scaled version is used.  I could not resist to kick some PresContexts.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 207175 [details] [diff] [review]
patch

Looks reasonable to me, but I'd really like roc to glance at this, since he understands the distinction between PixelsToTwips() and ScaledPixelsToTwips() better than I do.  I'm seriously wondering why the former even exists....

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I get confused too, but roughly, ScaledPixelsToTwips maps device pixels to twips, GetPixelsToTwips maps CSS pixels to twips.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No wait, it's the other way around!

Comment 28

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  Sounds like this patch is doing the right thing, then.

Comment 29

[Bernd]

Attachment: comments

comment 25 and 26 suggest that the code would not suffer from a more explicit comment.

Comment 30

[Bernd]

re comment 23, yes the bug is old, see bug 29869

Comment 31

[Bernd]

fixed checked in, jag did find an error that I corrected in the checkin.