Gábor Stefanik Reporter
	Description • 17 years ago

If a large object has a -moz-border-radius larger than 50% or its equivalent in other units, the border's radius gets clamped to 50%, but not the background, resulting in a serious mess. On some of the "circles", scrolling has an effect on the appearance.

Reproducible: Always

Steps to reproduce:
Open the attached testcase.

Actual results:
The -moz-border-radii are interpreted for the background as they are, resulting in strange errors on the page.

Expected results:
The -moz-border-radii are interpreted as 50% both for the border and the background, resulting in all 4 tests being rendered as circles. Scrolling doesn't change the appearance of the page.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 1 • 17 years ago

Whoops, I didn't do the same kind of clamping to the border radii for backgrounds as I do for borders themselves.  I have a fix, but I need to investigate how this interacts with inline elements that get split... I've put a testcase in the URL.

Firefox 2 ends up drawing a rounded border around each segment; if you remove the -moz-border-radius, there is no yellow border on the right side of the first ABC or the left side of the third, and not on either side of the middle -- as it should be.

Fx3 currently does the same, but the background of each of the 3 chunks is rounded; the problem is that PaintBackground doesn't get the same information about which sides to skip as the border painting methods do.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 2 • 17 years ago

(The patch that I have is worth taking anyway though -- as much as I think that the testcase rendering is pretty, especially the 100% case, I don't think we can call it a feature :)

	Gábor Stefanik Reporter
	Comment 4 • 17 years ago

That screenshot confirms this on Mac, so All/All.

	Jesse Ruderman
	Comment 5 • 17 years ago

Tests the range 0% to 180% for a square.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 6 • 17 years ago

Here's the fix.  Do the right clamping in reused code, and also fudge things so that backgrounds without a border get rendered right.

There's still one edge case:

data:text/html,<div style="-moz-border-radius: 50px; background: red; border: 30px solid blue; border-left: none; width: 200px; height: 200px;"></div>

Basically, it's hard to decide when to round the background and when not to.  We only have one -moz-border-radius property, and it's technically the border radius.  So if we don't have a border on one side, what do we do?  The code in there forces adjacent corners to have no radius, which I think is visually correct.

But that obviously breaks down when -moz-border-radius is used without a border, to just round a background.  So this patch here forces all borders to be at least 1, for the purpose of computing radii for backgrounds.  However, we then get into trouble when you actually have an element like the edge case above, where one side of the border is explicitly left off.  This is ok though, because the Fx2 behaviour was even more broken (we drew the background with a radius, but it stuck out into the border area, and the border was oddly cut off, etc.).

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 7 • 17 years ago

static void
+ComputePixelRadii(const nscoord *aTwipsRadii,
+                  const nsRect& outerRect,
+                  const nsMargin& borderMargin,
+                  PRIntn skipSides,
+                  nscoord twipsPerPixel,
+                  gfxFloat *oBorderRadii)

What naming convention(s) are you using here? :-)

This needs a comment.

I don't understand your approach to missing borders. Why introduce a discontinuity at border size zero? A 1-appunit border shouldn't render very differently to a 0 appunit border. I think in the style system we can tell the difference between a border of width 0 and 'none'. IMHO the latter should stop adjacent corners from rounding, but the former should not.

> But that obviously breaks down when -moz-border-radius is used without a
> border, to just round a background.

Should we allow this? Can we require people to apply border: 0 in that case?

	Alfred Kayser
	Comment 8 • 17 years ago

W3c says this on 'border-radius':
The border radius also causes the element's background to be rounded, even if the border is 'none'. 

So, when -moz-border-radius is defined for a corner, even if the corr. border is none or 0, the background of that corner needs to be rounded.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 9 • 17 years ago

Then it seems we shouldn't check the border width at all, just the passed-in aSkipSides parameter. This should just be telling us when an element was broken and it really doesn't have a left/right/top/bottom border.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 10 • 17 years ago

Forgot I had this patch still sitting around... this does better clamping, and doesn't special-case a border width of 0 for border-radius.  The only downside is that when an element with a border is split (and we get skipsides passed in), we'll round the background of each chunk seperately... this is what 2.0 did though, so it's not so bad.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 11 • 17 years ago

Comment on attachment 268961 [details] [diff] [review]
clamp, v2.0

ComputePixelRadii needs documentation.

	Gábor Stefanik Reporter
	Comment 12 • 17 years ago

When will this be finally checked in? Or is there still some other work going on about it?

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 13 • 17 years ago

Checked in (with documentation).

	Samuel Sidler (old account; do not CC)
	Comment 14 • 16 years ago

v.fixed

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre