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

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: zwol, Assigned: zwol)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla2.0b7
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [parity-IE] [parity-webkit] [parity-opera], URL)

Attachments

(3 attachments)

(Assignee)

Description

9 years ago
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?)
(Assignee)

Updated

9 years ago
Blocks: 431176

Comment 1

8 years ago
"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).
(Assignee)

Comment 2

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

Comment 3

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

Updated

7 years ago
Whiteboard: [parity-IE]
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
blocking2.0: --- → ?
Blocks: 569993
Keywords: dev-doc-needed
I think bug 382721 is pretty important and should be added to this list of "must fix".
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).
(Assignee)

Comment 7

7 years ago
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.
blocking2.0: ? → beta2+
Bug hasn't been touched since 21st of June - what's the deal? Moving to betaN.
blocking2.0: beta2+ → betaN+
blocking2.0: betaN+ → beta5+

Comment 9

7 years ago
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.
(Assignee)

Updated

7 years ago
Depends on: 485501, 471643, 459144
(Assignee)

Comment 10

7 years ago
I no longer work for Mozilla, I am deassigning myself from bugs I have no intention of working on as a volunteer.
Assignee: zackw → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 589700
Duplicate of this bug: 589701
Assignee: nobody → dbaron
Duplicate of this bug: 589704
Duplicate of this bug: 589703
Duplicate of this bug: 589702
Duplicate of this bug: 589706
Duplicate of this bug: 589705
Duplicate of this bug: 589707
Duplicate of this bug: 589708
Duplicate of this bug: 589697
Duplicate of this bug: 589698
Duplicate of this bug: 589699
Duplicate of this bug: 589695
Duplicate of this bug: 589694
Duplicate of this bug: 589693
Duplicate of this bug: 589692
Duplicate of this bug: 589690
Duplicate of this bug: 589689
Duplicate of this bug: 589688
Duplicate of this bug: 589710
Duplicate of this bug: 589709
Duplicate of this bug: 589683

Comment 33

7 years ago
ietestcenter url is http://samples.msdn.microsoft.com/ietestcenter/css3/bordersbackgrounds/border-radius-shorthand-001.htm
Duplicate of this bug: 589711
Duplicate of this bug: 589712

Comment 36

7 years ago
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

7 years ago
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).
(Assignee)

Comment 38

7 years ago
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

7 years ago
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.
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.
(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

7 years ago
If it's important to pass these test cases, then why close the bugs that are the only place they are kept?
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.
(Assignee)

Comment 44

7 years ago
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.
(Assignee)

Comment 45

7 years ago
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.
Assignee: dbaron → zackw
Status: NEW → ASSIGNED
Attachment #468595 - Flags: review?(dbaron)
(Assignee)

Comment 46

7 years ago
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.
Attachment #468596 - Flags: review?(dbaron)
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

7 years ago
> 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.
(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.
(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

Updated

7 years ago
Whiteboard: [parity-IE] → [parity-IE] [parity-webkit] [parity-opera]
--> beta6+
blocking2.0: beta5+ → beta6+
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.
Attachment #468595 - Flags: review?(dbaron) → review+
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.
Attachment #468596 - Flags: review?(dbaron) → review+
Alias code went away here:
http://hg.mozilla.org/mozilla-central/rev/e731e7196ba9
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.
Created attachment 473291 [details] [diff] [review]
add aliases
Attachment #473291 - Flags: review?(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.
Attachment #473291 - Flags: review?(bzbarsky) → review+

Comment 58

7 years ago
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...
http://hg.mozilla.org/mozilla-central/rev/c1cc7b565dc7
http://hg.mozilla.org/mozilla-central/rev/c954983caff1
http://hg.mozilla.org/mozilla-central/rev/8adb2f64c138
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6

Updated

7 years ago
Blocks: 594988

Updated

7 years ago
Blocks: 594991
The docs were updated last night by others. That makes me happy.
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 61

7 years ago
The shorthand page (border-radius) is good, but the individual border-*-radius pages still need quite a bit of polish.
Keywords: dev-doc-complete → dev-doc-needed

Comment 62

7 years ago
> 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")
(Assignee)

Comment 63

7 years ago
(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.
Yep, that's perfect.
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
Keywords: dev-doc-needed → dev-doc-complete

Updated

7 years ago
Duplicate of this bug: 602453

Comment 67

7 years ago
Is Bug 603415 fallout from this patch? regression?
(Assignee)

Comment 68

7 years ago
(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.
You need to log in before you can comment on or make changes to this bug.