Last Comment Bug 451134 - change -moz-border-radius* properties to css3-background names
: change -moz-border-radius* properties to css3-background names
Status: RESOLVED FIXED
[parity-IE] [parity-webkit] [parity-o...
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal with 7 votes (vote)
: mozilla2.0b7
Assigned To: Zack Weinberg (:zwol)
:
:
Mentors:
http://dev.w3.org/csswg/css3-backgrou...
: 589683 589688 589689 589690 589692 589693 589694 589695 589697 589698 589699 589700 589701 589702 589703 589704 589705 589706 589707 589708 589709 589710 589711 589712 602453 (view as bug list)
Depends on: 450652 459144 471643 485501
Blocks: border-radius ietestcenter 569993 594988 594991
  Show dependency treegraph
 
Reported: 2008-08-18 18:44 PDT by Zack Weinberg (:zwol)
Modified: 2010-10-17 12:56 PDT (History)
41 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta7+


Attachments
patch (1/2): mechanical changes (224.31 KB, patch)
2010-08-23 21:06 PDT, Zack Weinberg (:zwol)
dbaron: review+
Details | Diff | Splinter Review
patch (2/2): by-hand changes (15.82 KB, patch)
2010-08-23 21:12 PDT, Zack Weinberg (:zwol)
dbaron: review+
Details | Diff | Splinter Review
add aliases (2.67 KB, patch)
2010-09-08 16:56 PDT, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
bzbarsky: review+
Details | Diff | Splinter Review

Description Zack Weinberg (:zwol) 2008-08-18 18:44:38 PDT
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 2009-06-30 14:25:13 PDT
"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) 2009-06-30 16:14:26 PDT
It waits for the spec to hit CR, like it says in the description.
Comment 3 Sergiu Dumitriu 2010-02-18 06:05:40 PST
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-7 (busy September 14-25) 2010-06-04 17:35:38 PDT
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) 2010-06-21 09:38:28 PDT
I think bug 382721 is pretty important and should be added to this list of "must fix".
Comment 6 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-06-21 11:27:48 PDT
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) 2010-06-21 13:24:44 PDT
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] 2010-07-19 19:53:58 PDT
Bug hasn't been touched since 21st of June - what's the deal? Moving to betaN.
Comment 9 Ryan Jones 2010-08-11 13:58:47 PDT
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) 2010-08-18 09:41:55 PDT
I no longer work for Mozilla, I am deassigning myself from bugs I have no intention of working on as a volunteer.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:13:32 PDT
*** Bug 589700 has been marked as a duplicate of this bug. ***
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:14:33 PDT
*** Bug 589701 has been marked as a duplicate of this bug. ***
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:15:04 PDT
*** Bug 589704 has been marked as a duplicate of this bug. ***
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:15:09 PDT
*** Bug 589703 has been marked as a duplicate of this bug. ***
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:16:07 PDT
*** Bug 589702 has been marked as a duplicate of this bug. ***
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:17:10 PDT
*** Bug 589706 has been marked as a duplicate of this bug. ***
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:17:14 PDT
*** Bug 589705 has been marked as a duplicate of this bug. ***
Comment 18 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:17:40 PDT
*** Bug 589707 has been marked as a duplicate of this bug. ***
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:18:24 PDT
*** Bug 589708 has been marked as a duplicate of this bug. ***
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:18:28 PDT
*** Bug 589697 has been marked as a duplicate of this bug. ***
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:18:34 PDT
*** Bug 589698 has been marked as a duplicate of this bug. ***
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:18:38 PDT
*** Bug 589699 has been marked as a duplicate of this bug. ***
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:20:53 PDT
*** Bug 589695 has been marked as a duplicate of this bug. ***
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:02 PDT
*** Bug 589694 has been marked as a duplicate of this bug. ***
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:13 PDT
*** Bug 589693 has been marked as a duplicate of this bug. ***
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:23 PDT
*** Bug 589692 has been marked as a duplicate of this bug. ***
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:33 PDT
*** Bug 589690 has been marked as a duplicate of this bug. ***
Comment 28 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:44 PDT
*** Bug 589689 has been marked as a duplicate of this bug. ***
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:21:55 PDT
*** Bug 589688 has been marked as a duplicate of this bug. ***
Comment 30 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:22:30 PDT
*** Bug 589710 has been marked as a duplicate of this bug. ***
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:22:35 PDT
*** Bug 589709 has been marked as a duplicate of this bug. ***
Comment 32 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:32:04 PDT
*** Bug 589683 has been marked as a duplicate of this bug. ***
Comment 34 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:38:01 PDT
*** Bug 589711 has been marked as a duplicate of this bug. ***
Comment 35 Boris Zbarsky [:bz] (still a bit busy) 2010-08-22 20:38:24 PDT
*** Bug 589712 has been marked as a duplicate of this bug. ***
Comment 36 Darxus 2010-08-22 20:40:26 PDT
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 2010-08-22 21:02:20 PDT
Please don't close this bug without re-testing all of http://samples.msdn.microsoft.com/ietestcenter/#css3borders

Feel free to email me (darxus@chaosreigns.com).
Comment 38 Zack Weinberg (:zwol) 2010-08-23 10:08:39 PDT
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 2010-08-23 11:42:31 PDT
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] 2010-08-23 12:03:54 PDT
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] 2010-08-23 12:06:24 PDT
(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 2010-08-23 12:23:19 PDT
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] 2010-08-23 13:07:35 PDT
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) 2010-08-23 20:30:08 PDT
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) 2010-08-23 21:06:31 PDT
Created attachment 468595 [details] [diff] [review]
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) 2010-08-23 21:12:32 PDT
Created attachment 468596 [details] [diff] [review]
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) 2010-08-24 13:50:44 PDT
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. 2010-08-24 15:23:20 PDT
> 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) 2010-08-24 16:05:20 PDT
(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] 2010-08-24 18:42:54 PDT
(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] 2010-08-30 08:06:34 PDT
--> beta6+
Comment 52 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-09-08 16:10:10 PDT
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-7 (busy September 14-25) 2010-09-08 16:10:53 PDT
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-7 (busy September 14-25) 2010-09-08 16:14:21 PDT
Alias code went away here:
http://hg.mozilla.org/mozilla-central/rev/e731e7196ba9
Comment 55 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-09-08 16:18:50 PDT
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-7 (busy September 14-25) 2010-09-08 16:56:32 PDT
Created attachment 473291 [details] [diff] [review]
add aliases
Comment 57 Boris Zbarsky [:bz] (still a bit busy) 2010-09-08 17:38:46 PDT
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 2010-09-09 09:26:20 PDT
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 60 Eric Shepherd [:sheppy] 2010-09-10 11:08:56 PDT
The docs were updated last night by others. That makes me happy.
Comment 61 Zack Weinberg (:zwol) 2010-09-10 12:10:03 PDT
The shorthand page (border-radius) is good, but the individual border-*-radius pages still need quite a bit of polish.
Comment 62 j.j. 2010-09-10 23:07:40 PDT
> 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) 2010-09-11 09:29:19 PDT
(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] 2010-09-14 05:44:25 PDT
Yep, that's perfect.
Comment 65 Eric Shepherd [:sheppy] 2010-10-04 13:36:05 PDT
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 66 John P Baker 2010-10-07 02:14:59 PDT
*** Bug 602453 has been marked as a duplicate of this bug. ***
Comment 67 Philip Chee 2010-10-17 11:42:57 PDT
Is Bug 603415 fallout from this patch? regression?
Comment 68 Zack Weinberg (:zwol) 2010-10-17 12:56:17 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.