451134 - change -moz-border-radius* properties to css3-background names

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Zack Weinberg (:zwol)]

After css3-background reaches CR status we should remove the vendor prefix from the -moz-border-radius* properties and also change their name structure to match the spec, e.g. -moz-border-radius-topleft to border-top-left-radius.

Naturally this should only happen after bug 450652 lands, so that we actually implement the spec.

(Tangential question - what should happen to the -moz-outline-radius* properties?)

Comment 1

[Faruk Ates]

"Naturally this should only happen after bug 450652 lands, so that we actually
implement the spec."

Since that bug has landed, what's the status of this? I was expecting this to go into Firefox 3.5 before it hit final, but apparently it didn't make it.

Is this now going to wait on the spec to hit CR so that it can be implemented sans prefix, or will it be adjusted with the prefix? (I'd much rather see the latter, because who knows how long until the spec hits CR).

Comment 2

[Zack Weinberg (:zwol)]

It waits for the spec to hit CR, like it says in the description.

Comment 3

[Sergiu Dumitriu]

The specification is in the CR state now, see http://www.w3.org/TR/2009/CR-css3-background-20091217/

Comment 4

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

I think we should drop the -moz- prefix from the border-radius properties for our next release.  They've been "official" since 17 December 2009, our support is pretty good, and Web authors use them all over the place with prefixes already, so keeping the prefix doesn't really do authors much good and getting to the point where authors don't need the -moz-* variants sooner is a good thing.

So I think we should:
 * figure out which of the dependencies of bug 431176 we need to fix before removing the prefix.  (I tend to think just bug 485501, bug 459144, and bug 471643, which should all be relatively straightforward.)
 * drop the prefix and fix the names of the properties as per comment 0

Comment 5

[Anthony Ricaud (:rik)]

I think bug 382721 is pretty important and should be added to this list of "must fix".

Comment 6

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

I don't, partly because I don't think it's realistic to fix that in time for the next release (although I could be wrong, maybe Zack thinks it is).

Comment 7

[Zack Weinberg (:zwol)]

If we had an algorithm to draw dotted/dashed curved corners it would be a simple matter of programming, that could be assigned to an intern or somewhat.  But several different people have tried and failed to come up with that algorithm, so it remains a hard problem, and I agree with dbaron that it is not realistic to fix that for the next release.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Bug hasn't been touched since 21st of June - what's the deal? Moving to betaN.

Comment 9

[Ryan Jones-Ward [:sciguyryan]]

Per http://www.css3.info/preview/rounded-border/ it seems that Gecko is the only engine still using the prefix. Does anyone happen to know if this will be done for the final Firefox 4.0 release since these guys usually take some time to update their pages - would be better to notify them sooner rather than later probably.

Comment 10

[Zack Weinberg (:zwol)]

I no longer work for Mozilla, I am deassigning myself from bugs I have no intention of working on as a volunteer.

Comment 33

[Darxus]

ietestcenter url is http://samples.msdn.microsoft.com/ietestcenter/css3/bordersbackgrounds/border-radius-shorthand-001.htm

Comment 36

[Darxus]

Boris:  Are you sure all 24 test cases you just closed as duplicates of this bug are all the same problem in mozilla, or should they be left open as blocking this bug?

Comment 37

[Darxus]

Please don't close this bug without re-testing all of http://samples.msdn.microsoft.com/ietestcenter/#css3borders

Feel free to email me ([e-mail]).

Comment 38

[Zack Weinberg (:zwol)]

Darxus: We appreciate the test cases, but the way you submitted them makes a whole bunch of extra work for us.  Would you be interested in helping us absorb that complete test suite into our automated tests?  It looks like they could be converted to reftest format (see https://developer.mozilla.org/en/Creating_reftest-based_unit_tests ) relatively easily.

Comment 39

[Darxus]

Zack:  That looks like quite a lot more work for me, on a task that nobody has touched in the five months since the ietestcenter bug was opened.  I'm probably less than half way through, and the bugs that have been marked as duplicates of this one are a small portion of what I've gone through so far.  There are 2,138 tests.

The most standards compliant browser available is still from Microsoft, because nobody else has been willing to touch this.  

And it really bothers me that groups of them are being closed as duplicates of a single bug.  If this bug gets fixed and closed without somebody taking the time to go back through the bugs that have been closed as duplicates, then there are probably going to be several test cases that still fail.  They're not duplicates.  I think it makes a lot more sense for them to be open and listed as depending on this bug, instead of duplicates of it.

Comment 40

[Daniel Holbert [:dholbert]]

They are duplicates in the sense that this bug is the immediate reason that those tests currently fail.

If one of Microsoft's border-radius test is demonstrated to fail after Mozilla switches to border-radius syntax, or after the test has been modified to use "-moz-border-radius" syntax, then that would be worth filing a bug on.

Otherwise, there's not much point in having so many open bugs on testcases that depend on a syntax that Mozilla doesn't yet support.

Comment 41

[Daniel Holbert [:dholbert]]

(Definitely agreed RE it being important/useful to verify that we pass all those tests once we switch to "border-radius" syntax, though.)

Comment 42

[Darxus]

If it's important to pass these test cases, then why close the bugs that are the only place they are kept?

Comment 43

[Daniel Holbert [:dholbert]]

It's just not worth maintaining a separate bug for every individual test in a test-suite, particularly when most of those tests will presumably start passing as soon as we flip the switch here.

We don't have individual bugs filed for every single SVG 1.1 Test Suite test (each of which failed in Mozilla at some point in the past).  Instead, there are bugs on specific tests & groups of tests that fail in specific (and distinct) ways.  Currently, most (if not all) of the MS tests fail in the *same* way -- due to a syntax mismatch -- and it's not very useful to track that exact same failure individually for each of the tests.

What'd be more useful is a "pass the MS border-image testsuite" meta-bug, much like bug 512501, which would track any outstanding issues preventing us from passing those tests.  Currently this bug would be the main thing blocking that meta-bug, and other issues could be filed & set to block it as well, as they're discovered.

Comment 44

[Zack Weinberg (:zwol)]

FWIW, I patched up a local build to support the official border-*radius properties instead of the -moz-border-radius* properties, and with that build + the patch for bug 471643, I _think_ all the remaining ietestcenter css3-background failures are to do with background-clip and/or box-shadow, not border-radius.  Some of them, the success criterion is less than clear.

Comment 45

[Zack Weinberg (:zwol)]

Attachment: patch (1/2): mechanical changes

In the interest of being ready to flip this switch once the dependent bugs get fixed, I have done up patches that do the actual switch-flipping.  This first piece is _wholly_ mechanical: it is exactly what you get if you do this at the top level of the repository:

$ grep -rlEZe '[-_][Mm]oz[-_][Bb]order[-_][Rr]adius([-_]([Tt]op|[Bb]ottom)([Rr]ight|[Ll]eft))?' * | 
  xargs -0 perl -pi~ -e \
    's/-moz-border-radius-(top|bottom)(left|right)/border-${1}-${2}-radius/g;
     s/_moz_border_radius_(top|bottom)Right/border_${1}_right_radius/g;
     s/_moz_border_radius_(top|bottom)Left/border_${1}_left_radius/g;
     s/-moz-border-radius/border-radius/g;
     s/_moz_border_radius/border_radius/g;'

In the likely event of this patch bitrotting by the time it comes to apply it, I recommend repeating the above rather than trying to update it by hand.

Comment 46

[Zack Weinberg (:zwol)]

Attachment: patch (2/2): by-hand changes

This second piece was done by hand, and makes changes that were too inconsistent to script.  The large deletion-and-reinsertion blocks in nsIDOMCSS2Properties.idl and nsCSSPropList.h are to keep things in mostly-alphabetical order.

Two high-level notes about these patches: I did not include compatibility aliases for the -moz- properties, but I kinda think we should; and I did not touch the -moz-outline-radius* properties, which AFAIK is the right thing (outline-radius not being standardized at all) but does mean that we now have 'border-top-left-radius' but '-moz-outline-radius-topleft', which is ew.

Comment 47

[Christopher Blizzard (:blizzard)]

In general we've maintained support for -moz prefixed stuff for one release past moving to final properties.  Border radius is probably more widely deployed than previous properties, so I suspect that we'll have to hold onto this one a little longer than previously deprecated properties.

Comment 48

[j.j.]

> In general we've maintained support for -moz prefixed stuff for one release
> past moving to final properties.  

No, we haven't. Examples:

"opacity"        since Gecko 1.7   (Fx 0.9)
"-moz-opacity"   dropped     1.9.1 (Fx 3.5)

cursor:progress  since Gecko 1.7.1 (Fx 1.0)
"-moz-spinning"  dropped     2.0   (Fx 4.0)

"outline"        since Gecko 1.8   (Fx 1.5)
"-moz-outline"   dropped     1.9.2 (Fx 3.6)


> Border radius is probably more widely deployed than previous properties,
> so I suspect that we'll have to hold onto
> this one a little longer than previously deprecated properties.

No, please. A short confusion for web authors is better than a long one, especially with the different syntax we have here.

They will not start using "border-radius" as long as "-moz-border-radius" works,
or they will code:
  -moz-border-radius-topleft: 20px;
       border-radius-topleft: 20px; /*instead of "border-top-left-radius" */
see this working and think it's future proof.

Comment 49

[philippe (part-time)]

(In reply to comment #48) 
> They will not start using "border-radius" as long as "-moz-border-radius"
> works,
> or they will code:
>   -moz-border-radius-topleft: 20px;
>        border-radius-topleft: 20px; /*instead of "border-top-left-radius" */
> see this working and think it's future proof.

I seriously doubt they'll do that, anno 2010. They'll notice their syntax doesn't work in other browsers (most authors do check in more than Gecko and IE nowadays).

BTW some of the -moz-background-*** properties introduced in Gecko 1.9.2 had their prefixes dropped for Gecko 2.0, without continuing support for the prefixed one.

Comment 50

[Masatoshi Kimura [:emk]]

(In reply to comment #49)
> I seriously doubt they'll do that, anno 2010. They'll notice their syntax
> doesn't work in other browsers (most authors do check in more than Gecko and IE
> nowadays).
Which other browsers? Internet Explorer will support border-radius without prefix since 9.0. They had not supported (-ms-)border-radius even with prefix in 8.0 or earlier. Opera supported border-radius without prefix since 10.50. They had not supported (-o-)border-radius even with prefix in earlier versions. WebKit supports border-radius without prefix since 2009-07.
https://bugs.webkit.org/show_bug.cgi?id=27578

Comment 51

[Mike Beltzner [:beltzner, not reading bugmail]]

--> beta6+

Comment 52

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

Comment on attachment 468595 [details] [diff] [review]
patch (1/2): mechanical changes

I'll repeat this, and I'm just reviewing the script rather than the whole thing.

Comment 53

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

Comment on attachment 468596 [details] [diff] [review]
patch (2/2): by-hand changes

This looks fine.

I think I agree about the alias.  I'll dig up our old property alias code and add these in.

Comment 54

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

Alias code went away here:
http://hg.mozilla.org/mozilla-central/rev/e731e7196ba9

Comment 55

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

To be clear, the aliases would be for this release cycle only; the idea would be to remove them on mozilla-central after 2.0 branches.

Comment 56

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

Attachment: add aliases

Comment 57

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 473291 [details] [diff] [review]
add aliases

> +      if (nsCRT::strcasecmp(prop.get(), alias->name) == 0) {

That could be:

  if (prop.LowercaseEqualsASCII(alias->name)) {

right?  Either way, though.

Comment 58

[Alfred Kayser]

For themes that want to keep compatibility with FF3 and FF2 dropping the aliases will be a problem, as we then need to do both -moz-border-radius-* and the new border-radius-* style rules, which generally causes a lot of complaints in the Error Console, and addons reviewers sometimes reject extensions because of these...

Comment 59

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

http://hg.mozilla.org/mozilla-central/rev/c1cc7b565dc7
http://hg.mozilla.org/mozilla-central/rev/c954983caff1
http://hg.mozilla.org/mozilla-central/rev/8adb2f64c138

Comment 60

[Eric Shepherd [:sheppy]]

The docs were updated last night by others. That makes me happy.

Comment 61

[Zack Weinberg (:zwol)]

The shorthand page (border-radius) is good, but the individual border-*-radius pages still need quite a bit of polish.

Comment 62

[j.j.]

> the individual border-*-radius pages still need quite a bit of polish.

I'll look in this next week, if nobody else does it before. 

(It's not worth to have duplicate content for each corner, will concentrate on  "border-top-left-radius")

Comment 63

[Zack Weinberg (:zwol)]

(In reply to comment #62)
> > the individual border-*-radius pages still need quite a bit of polish.
> 
> I'll look in this next week, if nobody else does it before. 

Thanks.

> (It's not worth to have duplicate content for each corner, will concentrate on 
> "border-top-left-radius")

How about you have the other three pages redirect to that one, then, and discuss all four properties there.

Comment 64

[Eric Shepherd [:sheppy]]

Yep, that's perfect.

Comment 65

[Eric Shepherd [:sheppy]]

The docs appear to be fully updated now:

https://developer.mozilla.org/en/CSS/border-top-left-radius

and

https://developer.mozilla.org/en/CSS/border-radius

Comment 67

[Philip Chee]

Is Bug 603415 fallout from this patch? regression?

Comment 68

[Zack Weinberg (:zwol)]

(In reply to comment #67)
> Is Bug 603415 fallout from this patch? regression?

The problem there was *exposed* by this patch because the test case only used the css3-background name border-top-left-radius, but the bug looks unrelated to me.  I've followed up there.