Closed Bug 657143 Opened 13 years ago Closed 13 years ago

Reorder mature CSS properties in nsComputedDOMStyle.cpp

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla6

People

(Reporter: Kwan, Assigned: Kwan)

References

Details

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

Originally the CSS properties in nsComputedDOMStyle.cpp were listed in alphabetical order, and then experimental properties prefixed with -moz- were kept separate at the bottom.  Several of these properties have now matured/stabilised enough to drop the prefix and become fully supported, yet they are still in the list of experimental properties.  While this may not seem a particularly big deal there is one area where is has a noticeable effect: DOM Inspector's computed style panel, which seems to be ordered by this file.  As such when inspecting an elements style it often becomes annoying trying to locate properties that are not where you would expect to find them, and then having to scroll up and down to check properties that should be next to each other (ie background-*).

Reproducible: Always

Steps to Reproduce:
1. Install the DOM Inspector
2. Inspect an element in a web page
3. Check out its computed style
4. Properties are not where expected/have to scroll up and down to check different background-* properties

Actual Results:  
mature properties should be in alphabetical order, and then -moz- properties below them in their own order

Expected Results:  
Some matures properties are mixed in among the -moz- ones, and some of the -moz- ones are not in order within -moz-

Other improvements:
*Bump the SVG specific styles down into their own block with a comment at the top so future non-SVG styles are not mixed in with them (as currently happened with -moz-text-blink and -moz-text-decoration-*), plus they are no longer experimental anyway
*Change CSS2 to just CSS in the comment block at the top of the list, since it is no longer purely CSS2 properties but CSS3 as well

Have preliminary patch for m-c, will attach shortly
I assume its too late to take a similar patch for aurora/fx5, but if not I'd be happy to put one together
Severity: trivial → normal
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment on attachment 532465 [details] [diff] [review]
Reorder properties in nsComputedDOMStyle.cpp as described

r=me

> I assume its too late to take a similar patch for aurora/fx5

Yep.

Please let me know if you want me to push the patch for you, ok?
Attachment #532465 - Flags: review?(bzbarsky) → review+
Yes that would be great thank you.
Whiteboard: fixed-in-cedar
Pushed:
http://hg.mozilla.org/mozilla-central/rev/cf2a754bf3cb
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla6
fwiw, we could add a test for this, since this ordering is detectable from script.
Flags: in-testsuite- → in-testsuite?
Reopened as I missed some singletons and the new -moz-orient was put into the wrong list pretty much straight after :D. Patch with test incoming.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Okay figured out your test framework and made this, which also showed some I'd missed as they weren't obviously out of place (except ime-mode, that's just my fail) so fixed those as well.  roc kindly tested it for me on a local build of his (and fixed a couple of my mistakes) where it errored on the new -moz-orient in the SVG list, but the error message wasn't that helpful so I reworked it to be better in that case, and fixed -moz-orient.  I've tested it as best I can locally but can't build so can't verify that it definitely gives the right error for -moz- in SVG, but I've looked at the code pretty hard and don't see why it wouldn't.
Attachment #533456 - Flags: review?(bzbarsky)
Comment on attachment 533456 [details] [diff] [review]
add test and fix some missed props and new -moz-orient

>+    if (prop == 'clip-path') {

Need to push that into svgA.

Other than that, looks good.
Attachment #533456 - Flags: review?(bzbarsky) → review+
Fixed comments.
Ah, of course it does.  Thank you.
Attachment #533456 - Attachment is obsolete: true
Keywords: checkin-needed
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed in cedar]
Pushed:
http://hg.mozilla.org/mozilla-central/rev/abbf75ba7b59
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Verified this on Aurora earlier.  Thank you very much Boris, Robert and Mounir for all your help!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.