The inside and outside edges of double borders are not always even. Often the different sides are not even consistent. I would recommend the following algorithm, since it works at small px values, which are currently also a problem (this refers to width in px): If width is a multiple of 3 : split evenly among border/noborder/border If width is two more than a multiple of 3: make the gap 1px smaller than either side. There are two alternatives for the third case: 1) If width is one more than a multiple of 3: make the outer border 1px bigger than the gap or the inner border (this is what MSIE does, but I don't like it very much either. On second though, however, I guess I prefer it to the following.) 2) If width is one more than a multiple of 3: make the gap 2px smaller than either side (this has the disadvantage that the gap for a 4px border would be 0px, while the gap for a 3px border would be 1px) See the test case above (it should be useful).

Actually, choice two is basically out, because it yields what would end up being a 3px solid border for the 1px case. I think choice 1 is really the only way to do it.

Actually, there is another way 3) If width is one more than a multiple of 3: make the gap 1px larger than it otherwise would be, and handle 1px as a special case. This is probably the best way, even though the gap sizes progress (starting with 1px: 0,0,1,2,1,2,3,2,3,4,...) - the trick is to make the 1px case yield a 0px gap and not a 1px gap. This is done by Opera.

You seem to have chosen method 3, except that you haven't handled 1px as a special case yet. So most of the issues are fixed for borders in px. However, thin, medium, and thick still act very strangely. It would seem to me that if you just internally converted them to px then they would work fine. (Or are they in some other unit that doesn't work as well?) Perhaps if you converted to px before doing some of the calculations?

Setting all current Open/Normal to M4.

assigning ChrisD as qa

I think most of this was fixed by what peterl did to bug 1052. That is, edges are even now. I think the only real remaining issue is the 1px borders, unless you want to change the way you are doing the others.

I'm seeing uneven borders again, as I described originally.

To be more specific, I'm seeing problems on 1px, 2px, 4px, and 5px, on the top and bottom borders (plus, you're not correcting for 1px, which really makes the 1px problem on the sides).

Moving to M9

Moving to M10.

On WIN32 the double borders look OK. On Linux they are uneven. In addition the double border with width = 1 does not appear on WIN32. The problem is that nsCSSRendering::MakeSide creates a polygon where the coordinates x1, y1, x2, y2, x3, y3, x4, y4 are set to y1 = y2 = y3 = y4. This produces a line on GTK but fails to render anything on WIN32. Simply substituting a line in this case is not sufficient. Lines do not have the same pixel coverage as a polygon for the same set of vertices. The ending point of a line is not covered. The uneven borders are probably caused by rounding errors caused by the nscoord truncating the calculation of the edges. if (borderPart == BORDER_INSIDE) { outsideEdge = nscoord(outsideEdge * borderFrac + insideEdge * borderRest); outsideTL = nscoord(outsideTL * borderFrac + insideTL * borderRest); outsideBR = nscoord(outsideBR * borderFrac + insideBR * borderRest); } else if (borderPart == BORDER_OUTSIDE ) { insideEdge = nscoord(insideEdge * borderFrac + outsideEdge * borderRest); insideTL = nscoord(insideTL * borderFrac + outsideTL * borderRest); insideBR = nscoord(insideBR * borderFrac + outsideBR * borderRest); }

Moving to M11

Moving to M13

This problem is caused by not rounding off the twips to pixel boundaries. Don you already have code to fix this for rounded rects, so I'm re-assigning to you so you can fix it for regular border rects.

Transform fix has killed the line spacing problem. Half the problem. Giving it back to you Kevin for disposition on the other problems.

Moving to M14

Migrating from {css1} to css1 keyword. The {css1}, {css2}, {css3} and {css-moz} radars should now be considered deprecated in favour of keywords. I am *really* sorry about the spam...

Recap of the remaining problem: the double border with width = 1 does not appear on WIN32. The problem is that nsCSSRendering::MakeSide creates a polygon where the coordinates x1, y1, x2, y2, x3, y3, x4, y4 are set to y1 = y2 = y3 = y4. This produces a line on GTK but fails to render anything on WIN32. Simply substituting a line in this case is not sufficient. Lines do not have the same pixel coverage as a polygon for the same set of vertices. The ending point of a line is not covered.

Moving to M16. In nsRenderingContextWin :: DrawPolygon we need to look for the case described above and render the Polygon as a line with the proper pixel coverage.

Moving to M17

Moving to M18

This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.

Attempting to reassign bug.

Restoring original summary and component.

This is for Kevin to disposition and assign. Why was this taken off of future?

Not sure why this isn't still future

The only remaining problem is that the double border with width = 1 does not appear on WIN32. This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.

Adding correctness keyword

Use border width = 2 as a work around. Will fix in a future release

Marking nsbeta3-

*** Bug 54827 has been marked as a duplicate of this bug. ***

Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...

Updating summary based on Kevin's last few comments. What I see now (for <input> elements): - "1px double" or "thin double" makes a single, thin border appear, and only on the right. - "2px double" makes a single, thick border appear on the top, bottom, and left. A single, thin border appears on the right, just as with "1px double". - "3px double" does what you want it to do (two thin borders around the element).

still able to reproduce this bug on win32 buildID: 2001-10-22-18-0.9.4 (branch build) and macOS9.1 buildID: 2001-10-20-113.0.9.4 (branch build)

*** Bug 139193 has been marked as a duplicate of this bug. ***

*** Bug 22881 has been marked as a duplicate of this bug. ***

*** Bug 156003 has been marked as a duplicate of this bug. ***

I have a similar problem with the borders implementation in Mozilla. whenever I assign a border to the <body> tag Mozilla hacks the borders of all css elements in the page. please see http://jim.macdonald.org/test.html as an example.

I saved a local copy of http://jim.macdonald.org/test.html and removed the body borders. Nothing really changed, so the problem there isn't the body borders (it's actually the way the CSS is authored), and it's not really relevant to this bug anyway.

The "on Win32" really ought to come out the summary. Using Linux 1.2 on a P tag, "double black 1px" gives no border at all. nor does using "thin" instead. "double thick black" will give a double border. [Also checked in Galeon 1.2.6 on a different machine]

Expected: Three double-underlines paragraphs Result: [Linux 1.2] Only third paragraph has any border

*** Bug 114336 has been marked as a duplicate of this bug. ***

*** Bug 204378 has been marked as a duplicate of this bug. ***

*** Bug 273712 has been marked as a duplicate of this bug. ***

*** Bug 282929 has been marked as a duplicate of this bug. ***

taking.

fix. This patch adds forceSolid array to PaintBorder and optional boolean argument aFrameSolid to DrawSide.

Why do you need to add the parameter to DrawSide? You should be able to just change the passed-in borderStyle. + forceSolid[cnt] = (aBorderStyle.GetBorderStyle(cnt) == NS_STYLE_BORDER_STYLE_DOUBLE You should probably wrap this entire block of code (including the switch) in "if (aBorderStyle.GetBorderStyle(cnt) == NS_STYLE_BORDER_STYLE_DOUBLE)" + && sizeWidth/twipsPerPixel == 1); Maybe this should be sizeWidth/twipsPerPixel < 2

(In reply to comment #49) > Why do you need to add the parameter to DrawSide? You should be able to just > change the passed-in borderStyle. But I need to determinate if the border width is 1px. It is hard to do this from inside DrawSide. I can do this by doing (borderInside.x+borderInside.width)-(borderOutside.x+borderOutside.width) and stuff like this for every border... > + forceSolid[cnt] = (aBorderStyle.GetBorderStyle(cnt) == > NS_STYLE_BORDER_STYLE_DOUBLE > > You should probably wrap this entire block of code (including the switch) in "if > (aBorderStyle.GetBorderStyle(cnt) == NS_STYLE_BORDER_STYLE_DOUBLE)" > > + && sizeWidth/twipsPerPixel == 1); right > Maybe this should be sizeWidth/twipsPerPixel < 2 If it's possible that 0 > sizeWidth/twipsPerPixel > 2 and not 1 then sure. I'll send updated patch as soon as you agree (or not) on that parameter.

(In reply to comment #50) > (In reply to comment #49) > > Why do you need to add the parameter to DrawSide? You should be able to just > > change the passed-in borderStyle. > > But I need to determinate if the border width is 1px. It is hard to do this from > inside DrawSide. I can do this by doing > (borderInside.x+borderInside.width)-(borderOutside.x+borderOutside.width) and > stuff like this for every border... Instead of passing PR_TRUE for aForceSolid, you could just pass NS_STYLE_BORDER_STYLE_SOLID as the style when you call DrawSide.

patch addresses roc's notes.

Instead of the switch, it would be more readable to write this: nscoord widths = { border.bottom, border.left, border.top, border.right }; forceSolid = widths[cnt]/twipsPerPixel < 2; The comment should also be a bit more meaningful. I would just say // If a side needs a double border but will be less than two pixels, // force it to be solid (see bug 1781). With that, r+sr=roc

final version

umm, this one compiles :)

comment updated and removed trailing spaces.

+ /* + If a side needs a double border but will be less than two pixels, + force it to be solid (see bug 1781). + */ is that comment style the style of the file? seems to me like it almost exclusively uses the style roc used in his comment...

Comment on attachment 184171 [details] [diff] [review] patch v6 I'll fix comment style before checkin

I don't see reviews flagged on this patch. Can you get the appropriate reviews noted there? Thanks.

Comment on attachment 184171 [details] [diff] [review] patch v6 In comment 53 Roc gave me r+sr, next patches were only about comments, no changes in code.

Comment on attachment 184171 [details] [diff] [review] patch v6 sorry, didn't see that in the comments. Approved.

Comment on attachment 184171 [details] [diff] [review] patch v6 Checking in layout/base/nsCSSRendering.cpp; /cvsroot/mozilla/layout/base/nsCSSRendering.cpp,v <-- nsCSSRendering.cpp new revision: 3.261; previous revision: 3.260 done