379474 - Borders rendered incorrectly when using -moz-border-radius

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Andy Boze]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070501 SeaMonkey/1.5a Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070501 SeaMonkey/1.5a

I have a couple of example of borders not being rendered correctly when using -moz-border-radius. In one, using a dotted border, the bottom border does not render at all. When scrolling through the area, the bottom border starts to appear, and there are border artifacts rendered at the left side of the div, where there should be no border at all. In the other case, the background color of the text block bleeds past the rounded corners. This problem started with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070501 SeaMonkey/1.5a. It did not exist earlier.

Reproducible: Always

Steps to Reproduce:
1.View the example page in the attached test case.
2.
3.
Actual Results:  
Borders are missing or background colors bleed past border corners.

Expected Results:  
All borders should be rendered where expected, with no bleeding of backgrouond color.

This may be the same thing as Bug 377398, but since it was filed specifically on fieldset borders, I'm filing this as well.

Comment 1

[Andy Boze]

Attachment: Test case showing incorrectly rendered borders

Comment 2

[Andy Boze]

Attachment: Screen capture showing artifacts resulting from scrolling

Note that the bottom border, which is missing when the page is first rendered, begins to show, and there are artifacts at the left, where no border should be.

Comment 3

[Mats Palmgren (inactive)]

Since the problem started 20070501 I'm guessing regression from bug 368247.

Comment 4

[Jonathan Haas]

See also https://bugzilla.mozilla.org/attachment.cgi?id=263356 from bug 368247

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix incorrect border radius rendering

Fix for border radius, <hr> issue, and a few other things.

Comment 6

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

Comment on attachment 263841 [details] [diff] [review]
fix incorrect border radius rendering

>-                                     nsIRenderingContext& aContext,
>+                                    nsIRenderingContext& aContext,

Looks like an accidental space deletion.

>+    switch (skipSides) {
>+      case SIDE_BIT_TOP:
>+      case SIDE_BIT_TOP | SIDE_BIT_LEFT:
>+      case SIDE_BIT_TOP | SIDE_BIT_LEFT | SIDE_BIT_BOTTOM:
>+        currentSide = NS_SIDE_RIGHT;
>+        break;
>+      case SIDE_BIT_RIGHT:
>+      case SIDE_BIT_RIGHT | SIDE_BIT_TOP:
>+      case SIDE_BIT_RIGHT | SIDE_BIT_TOP | SIDE_BIT_LEFT:
>+        currentSide = NS_SIDE_BOTTOM;
>+        break;
>+      case SIDE_BIT_BOTTOM:
>+      case SIDE_BIT_BOTTOM | SIDE_BIT_RIGHT:
>+      case SIDE_BIT_BOTTOM | SIDE_BIT_RIGHT | SIDE_BIT_TOP:
>+        currentSide = NS_SIDE_LEFT;
>+        break;
>+      case SIDE_BIT_LEFT:
>+      case SIDE_BIT_LEFT | SIDE_BIT_BOTTOM:
>+      case SIDE_BIT_LEFT | SIDE_BIT_BOTTOM | SIDE_BIT_RIGHT:
>+        currentSide = NS_SIDE_TOP;
>+        break;
>+    }

This code is a little annoying since it seems to loop the reverse of the normal way through the sides (top, right, bottom, left).

>+        fprintf (stderr, "** Unhandled border style!! %d\n", borderRenderStyle);

This should be an NS_NOTREACHED.  If you want the parameter, you can use NS_NOTREACHED(nsPrintfCString(64, "** Unhandled border style!! %d\n", borderRenderStyle).get()), I think.


I didn't read through the big messy chunk of path creation code, but I'll trust you there.

r=dbaron

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 263841 [details] [diff] [review]
fix incorrect border radius rendering

argh, there's still a bug here

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix incorrect border radius rendering, v2

Ok, here's an updated patch.  Not much has changed, except for some fixing of border radius in the composite colors case.

dbaron, I fixed the issues you mentioned earlier, though I didn't understand the bit about the code drawing in a different order than usual -- top/right/bottom/left is the usual way, no?  All that code does it decides where to start the path along that route when taking into account sides that are skipped, so that the end caps are handled correctly -- but it will still draw t/r/b/l

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix incorrect border radius rendering, v2.0.1

Whoops, wrong patch last time, this is with the review comments addressed.

Comment 10

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

Comment on attachment 264112 [details] [diff] [review]
fix incorrect border radius rendering, v2.0.1

>-  ctx->SetDash(nsnull, 0, 0.0);
>+  ctx->SetDasih(nsnull, 0, 0.0);

Oops.

You also still need to address the fprintf and ordering comments from my previous review.

Comment 11

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

OK, never mind the ordering comment.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.

Comment 13

[Jeff Walden [:Waldo]]

I don't see anything testing dotted borders as described in comment 0, but I assume the problem wasn't limited to just dotted borders.  People should feel free to submit tests for these slightly different cases, as future-proofing, but it's not really a high priority.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/reftests/bugs&command=DIFF_FRAMESET&file=reftest.list&rev1=1.62&rev2=1.63&root=/cvsroot

Comment 15

[Andy Boze]

Attachment: Still a problem with rounded dotted borders

I'm just adding this to document that there's still a problem with dotted borders  when using -moz-border-radius-xxx

Comment 16

[Boris Zbarsky [:bzbarsky]]

Probably worth filing a followup bug on remaining issues....

Comment 17

[Andy Boze]

(In reply to comment #16)
> Probably worth filing a followup bug on remaining issues....
> 
OK, I've filed bug 380549 on the problem with dotted borders and rounded corners.

Comment 18

[Jonathan Haas]

Attachment: The size of the border-radius is still incorrect.

I believe the border-radius should be measured from the outer edge of the border.

Before the patch, it was measured from the center of the border, currently it looks like as if the border-radius is simply divided by two.

I think the formular should be 
drawn_center_of_border = border_radius - border_width / 2

Comment 19

[Jonathan Haas]

Attachment: Another issue

I'm too lazy to file separate bugs.

Comment 20

[Boris Zbarsky [:bzbarsky]]

> I'm too lazy to file separate bugs.

Then the issues in question probably won't get fixed...  The idea is one bug per issue, and this one is fixed, so no more work is going to happen in this bug.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Looks like both those testcases got fixed by the patch for bug 379505.