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)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla6
People
(Reporter: Kwan, Assigned: Kwan)
References
Details
Attachments
(2 files, 1 obsolete file)
14.30 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
7.78 KB,
patch
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #532465 -
Flags: review?(bzbarsky)
Assignee: nobody → moz-ian
Severity: trivial → normal
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment 2•13 years ago
|
||
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+
Assignee | ||
Comment 3•13 years ago
|
||
Yes that would be great thank you.
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Whiteboard: fixed-in-cedar
Comment 5•13 years ago
|
||
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
Comment 6•13 years ago
|
||
fwiw, we could add a test for this, since this ordering is detectable from script.
Updated•13 years ago
|
Flags: in-testsuite- → in-testsuite?
Assignee | ||
Comment 7•13 years ago
|
||
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 → ---
Assignee | ||
Comment 8•13 years ago
|
||
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 9•13 years ago
|
||
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+
Assignee | ||
Comment 10•13 years ago
|
||
Fixed comments. Ah, of course it does. Thank you.
Attachment #533456 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Comment 11•13 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/abbf75ba7b59
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Assignee | ||
Comment 12•13 years ago
|
||
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.
Description
•