188377 - Percentile table width (75%) is 1 pixel more than should.

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect, P4)

Description

[S Woodside]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.0.1) Gecko/20030108 Chimera/0.6+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.0.1) Gecko/20030108 Chimera/0.6+

This page renders fine in IE 5.2/mac. It shows up with the right sidebar below
the content in 1.2.1 and Chimera (what I'm submitting from)

Reproducible: Always

Steps to Reproduce:

Comment 1

[S Woodside]

Attachment: source

In case the author changes it.

Comment 2

[S Woodside]

Attachment: the css

Comment 3

[Boris Zbarsky [:bzbarsky]]

The two tables are aligned left and right and have widths 75% and 25%.  We're
computing them to not fit next to each other, so we move the one on the right
down.... (seems incorrect to me, unless it's just rounding error somewhere).

Comment 4

[Ruslan Ismailov]

I found, that table with width 75% have 1 pixel more than div with width 75%
(same effect I found on every percentage based tabs, but no in pixelized ones.

Comment 5

[Ruslan Ismailov]

Attachment: table with width=75% and div with same width have different width

I think this pixel is source of this bug.

Comment 6

[Ronald Tilby]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030108
I also see the problems (URL page bad rendering, and table vs. div have
different width) on Windows 2000 w/ 20030108 Nightly.
Suggest: Hardware: ALL, OS: ALL

Comment 7

[Ruslan Ismailov]

Attachment: Difference between table and div width

I found, that the source of this problem is rounding to integer for table
width.
Play with this testcase to lookup.

Comment 8

[Ruslan Ismailov]

I found source of incorrect rounding. It is in
layout/html/table/src/nsTableFrame, line 4218:

4195 nscoord 
4196 nsTableFrame::CalcBorderBoxWidth(nsIPresContext*          aPresContext,
4197                                  const nsHTMLReflowState& aState)
4198 {
4199   nscoord width = aState.mComputedWidth;
4200 
4201   if (eStyleUnit_Auto == aState.mStylePosition->mWidth.GetUnit()) {
4202     if (0 == width) {
4203       width = aState.availableWidth;
4204     }
4205     if (NS_UNCONSTRAINEDSIZE != aState.availableWidth) {
4206       width = aState.availableWidth;
4207     }
4208   }
4209   else if (width != NS_UNCONSTRAINEDSIZE) {
4210     nsMargin borderPadding = GetContentAreaOffset(*aPresContext, &aState);
4211     width += borderPadding.left + borderPadding.right;
4212   }
4213   width = PR_MAX(width, 0);
4214 
4215   if (NS_UNCONSTRAINEDSIZE != width) {
4216     float p2t;
4217     aPresContext->GetPixelsToTwips(&p2t);
4218     width = RoundToPixel(width, p2t);
4219   }
4220 
4221   return width;
4222 }

look, width rounding by default, so it runding up to nearest bigger interer. It
is incorrect, because table box rounding should be as div's, so correct line
should be:

    width = RoundToPixel(width, p2t, eRoundUpIfHalfOrMore)

Comment 9

[Ruslan Ismailov]

Attachment: Patch for table width rounding.

Patch for this line.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Even that wouldn't fix the accumulation and rounding problem for, say, four 25%
width divs, in some of the possible cases.

Also, might there be a reason the rounding was done the way it does?  The use of
default parameters makes me think there might not be, though.

Comment 11

[Ruslan Ismailov]

Well, maybe it would be nice to do all box-size rounding to down? This patch
fixed only table-div difference, so browser become work more reasonable with it.

Comment 12

[Ruslan Ismailov]

Attachment: Testcase for multy-tables rounding problem

Well, this is testcase for David case.

Comment 13

[Ruslan Ismailov]

Attachment: Testcase for David case

sorry, last testcase have an error.

Comment 14

[Ruslan Ismailov]

Seems, that 3 div's with any rounding's don't produce trouble. So I think that
proposex fix is all that need in this case. 

Comment 15

[Bernd]

will this also work when multiple cells are in a table, with computed fractional
pixels sizes? What I have in mind is a scenario like bug 48827. Ruslan to get a
review you will need to run the layout regression tests
http://lxr.mozilla.org/seamonkey/source/layout/doc/regression_tests.html .
Please note that those regression tests show only that you have taken care of
possible regressions they do not provide any information whether the patch
itself is correct. 

ps: I dont see that this bug should block 1.3b, 
does it crash :no 
      dataloss:no

Comment 16

[Ruslan Ismailov]

O'k, I start with tests. But what meaning of this under linux?

 Make a copy of gklayout.dll and gkcontent.dll in the dist directory. If you
would like to switch back, you will need them.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's not worth running the regression tests until we agree on the correctness of
the approach.

Furthermore, I don't even think they're necessary at all for this type of a patch.

Comment 18

[Ruslan Ismailov]

Well, I was lead by comment #2 -- rounding error. Right now <div> and <table>
use different form for rounding on perentile-pixelized calculation. Realy, I
think that this change could not invalidate other stuff, becase this is only one
place, when rounding occured. I could made a mistake, of course.

Comment 19

[Bernd]

Attachment: testcase with pixel rounding

Comment 20

[Bernd]

The patch will not make the table and the block percentage rounding identical.
But it will make the table rounding identical to IE-win. IE makes the transition
so that 176.5 is rounded up and 177.4 is rounded down. The blocks under mozilla
round 174.6 up and 175.5 down. 

Comment 21

[Bernd]

fix checked in