2.77 KB, text/plain
13.34 KB, patch
|Details | Diff | Splinter Review|
32.37 KB, patch
Hixie (not reading bugmail): review+
Chris Waterson: superreview+
|Details | Diff | Splinter Review|
ua.css contains many pseudos and properties which are presumably mozilla extensions used to implement features that the specs currently do not specify. These should all be marked, preferably by prefixing them with "-moz-", as has been done in some cases. Not doing so runs the risk of a clash with future specs. Current cases of unmarked mozilla-native pseudos in ua.css are: :out-of-date :scrollbar-look :scrollbar-arrow-look :scrollbar-thumb-look :scrolled-content :wrapped-frame :placeholder-frame :root :table :table-cell :table-column :table-column-group :table-outer :table-row :table-row-group :cell-content :fieldset-content :button-content :label-content :dropdown-visible :dropdown-hidden :combobox-text :combobox-textselected :combobox-textselectedfocus Current cases of unmarked mozilla-native properties in ua.css are: cell-spacing cell-padding
Is cell-spacing useless duplication of border-spacing, a CSS2 property?
Reassigning peterl's bugs to myself.
Accepting peterl's bugs that have a Target Milestone
Pushing my M15 bugs to M16
I'd be so happy if an intern volunteered to do it this summer :-) Pushed to M19.
opacity should also be -moz-opacity. Closing bug 39168 as DUP. David, Ian: we want this bug report to be a complete list of *all* mozilla CSS properties and pseudo classes that are extensions and should have the -moz- prefix. This bug was opened 14 months ago, and I note that opacity (bug 39168) is not on the list below. Could one of you please review our currently supported CSS properties and pseudo classes and make sure this list is 100% complete and up to date? We need to do that step in order to assess the feasibility and scheduling impact of making the change at this point. pierre: It looks like XUL and our app UI may use some of these features. (e.g. I'm thinking that these property names may be hardcoded into our XUL UI files numerous places.) Am I correct? If so, is there a way we can gracefully make the changeover without disrupting others? For example: could we do something like: step 1: add support for the -moz- names (leaving the old names also supported temporarily) step 2: do some kind of global search and replace on the codebase to change all instances of the old property names to the new ones (since we probably don't have the luxury of freezing the tree, doing a mass Perl batch operation, and then reopening it, this might need to be done pieces at a time) step 3: dropping support for the old names once we're sure the new ones are working and all usage instances within our code are out Since I don't know how the code is internally set up, my specific suggestions for approach and process might be way off base, but is there some way we could achieve the general goal of fixing this without breaking the world? ;-> Marking helpwanted,css1,css3. It would be great if someone would volunteer to take this on!
Copying in relevant comments from discussion in 39168. Adding jst and hyatt to cc: for jst to track DOM dependencies on this change and for hyatt to assess whether XUL has any dependencies on this change. ------- Additional Comments From Eric Krock ------- opacity is an under-development property in a CSS3 draft, but it's not finished yet. If we have our own opacity in FCS, developers write to it, and the final CSS3 draft specifies that opacity works in a different way than our initial implementation, we'll be forced at that point to (1) conform to CSS3 spec at the expense of breaking backward compatibility and existing content, or (2) not conform to the CSS3 spec. ------- Additional Comments From Ian Hickson 2000-05-15 02:32 ------- I would go even further and suggest we just _rename_ our "opacity" property to "-moz-opacity". Imagine if CSS3 does indeed introduce this property, but defines it totally differently to us (worse case scenario, for example: 100% means totally transparent, and 0% means totally opaque). Now anyone using "opacity" in the standards compliant way will find their documents look great in the next version of Mozilla, but DISAPPEAR in this version! Oops. ------- Additional Comments From Johnny Stenback 2000-05-27 10:57 ------- There's more to this than just renaming this property in the style system, the DOM CSS OM also has knowledge about the supported properties in the interface CSS2Properties, and opacity is in there (in our interface, not in the DOM spec). Should we either remove opacity from our DOM interface, or should we leave it in and add -moz-opacity? Either way is easy to do, just let me know what you think.
*** Bug 39168 has been marked as a duplicate of this bug. ***
Note that we know that a mass search-and-replace operation is _feasible_ at this stage, since we used exactly that system on the XHTML namespace id. It will require a lot of thought, however. As to finding a list of all our proprietary properties -- how can we do this? This should IMHO probably be done at least one beta before release, to iron out any problems, since this is at least a moderately risky process, if not too difficult to manage. Therefore, nominating nsbeta3. (I would suggest nsbeta3 hardstop since we want to know for _sure_ that this is ready for FCS.)
All CSS identifiers recognized (and all properties) are in a list somewhere...
The recognized identifiers are organized as following: nsCSSAtomList.h: pseudo-classes / pseudo-elements nsCSSPropList.h: properties nsCSSKeywordList.h: values
David, I doubt Pierre will have the time to do this. Is there any chance you could take on this bug, perhaps with his guidance?
OK, I guess I'll take it. Hopefully I'll have time. Pierre - the previous time I looked at the code, my impression was that there were constants generated from the names of the properties. It would be easier, IMO, if we changed the names of the properties without changing the constants. This could introduce problems later if we decide we want to support both the -moz- property and the new property with the same name. Do you think it's better to change the names of constants or leave them the same? There are also some issues about what working drafts we consider stable enough to use the names in the drafts...
Thanks David. I can think of at least two acceptable ways of fixing this: 1) (easier) just *add* the new mozilla-native pseudo classes (so we're able to evangelize developers and they can Do The Right Thing by using the -moz- versions in their content, avoiding future upward compatibility issues if we have to change the behavior of the non-"-moz-" properties per future releases of specs) but leave the existing ones in and supported (although deprecated for external use) in case there are internal dependencies within the product on them. 2) (harder but better if we can get away with it without causing regressions in the product) *change* the properties (and all instances of their usage within the product) from the "bad" names to the "good" names Justification of nsbeta3 nomination to PDT team: the CSS specification requires that vendor extensions to CSS must begin with a hyphen; there should also be some kind of prefix (in our case, "-moz-") identifying where these extensions come from. Currently, our implementation is out of compliance with the CSS spec in this way because the Mozilla CSS extensions listed above do not have the -moz- prefix. Not only is this a standards compliance bug, but if not fixed before FCS, it creates a very serious vulnerability for Mozilla's ability to be both standards-compliant and backwardly-compatible in the future. If these property names persist through RTM, not only would Mozilla have to retain the non-compliant property names in future releases in order to assure backward compatibility with content that had used them, but worse, the possibility exists that the CSS working group would define different semantics (behavior) in CSS3 for the same non-prefixed names we've used. This would create a situation where Mozilla would either have to break the standard (by retaining its current behavior for b.c. and ignoring what the standard says) or break backward compatibility with content that had adopted these Mozilla extensions (in order to comply with the standard). Because of the fact that external content and web applications will be written using these extensions (the developer-adoption "lock-in" effect in which we become unable to change our API due to external dependencies), it's crucial that we get this fixed before RTM, which means in nsbeta3 for safety. David Baron has taken on the work. Please call me if you're considering not approving this for nsbeta3.
Another property which should be marked with -moz- is 'behaviour', since as I understand it our implementation is very likely to differ from the BECSS spec.
... and that should be "behavior," because I believe American English spelling is the standard for property names in the W3C specs and mozilla.org, yes? (e.g. we support "color", not "colour") ;->
um, yes. Sorry, I keep forgetting to misspell these words.... ;-)
As per meeting with ChrisD today, taking QA. David: It would be good to try to get this done right at the start of the beta3 cycle, since this is comparatively high risk, and we will want as much time as possible to find bugs. Note to PDT: The risk is not to external, existing pages, but to internal demonstration pages and the chrome itself, so we should find problems very quickly, if any do occur.
David, in response to [2000-07-19 17:18], I agree it would be easier to keep the constants as they are but, in addition to potentially future incompatibilities, I'm afraid it may bring a bit of confusion for those who work on the code as well. Let's keep the current scheme...
Dave Hyatt says behavior should be renamed -moz-binding.
Here are some properties to rename: behavior (to -moz-binding) box-sizing (maybe -- it probably won't change though) counter-increment (there are some serious oddities in our support of this) counter-reset float-edge (very likely will change before CSS3 is released) key-equivalent (ui spec is in flux) opacity (see above) resizer (ui spec is in flux) user-focus (ui spec is in flux) user-input (ui spec is in flux) user-modify (ui spec is in flux) user-select (ui spec is in flux) Um, who's triaging David's bugs, btw?
Sorry, I meant who's [nsbeta3+]ing your bugs? You seem to have carefully avoided any +ing so far! ;-)
As long as you use your personal email address, you are an external contributor and you can work on whatever you want :-P You can triage the bugs yourself, mark nsbeta3 the ones you consider important, then send a mail to Marc and me so that we can throw in some more if you are not doomed enough. Marc is interim-manager, he's the one who "+" things.
Updating the Summary so it now include properties, changing OS and platform to All, adding me to CC.
I think it is *vital* that this gets fixed ASAP now. We are fast approaching the end of the nsbeta3 train, at which point something this big will never get approved. This is something for which I would *definitely* hold beta3. If David cannot do this at this point, then we need to find someone else who can. Pierre? You only have two open nsbeta3+ bugs... ;-)
Pierre - is it OK with you if I don't change the constants, at least not immediately? Changing the constants is something that can be done one at a time, later on, if desired. Tracking issues with the chrome will probably be enough to handle when I change the property names and values, so I'd rather not have to deal with tons and tons of source changes too, at the same time.
Marking + per ekrock.
Just to pipe in from the commercial side of things - it is _essential_ that whomever takes this on also do those changes in the commercial tree as well and not simply wait for things to break. This needs to be a proactive process, not reactive.
Andreww: That's all well and good if it is somebody from inside Netscape that does it, but if it is a non-Netscape Mozilla contributor who does the work then it is absolutely NOT their responsability to keep our internal tree up-to-date. Say IBM had a "commercial" tree of their own, like we do, would any changes we make to the Mozilla tree require us to fix the IBM tree as well? I think not.
I doubt this will affect the commercial build at all. YOu have no XBL over there, so -moz-binding won't affect you. That's the most prominent change out of all of these...
There are a handful of changes that need to be made (say, about 20) to the commercial tree, and I will make them. (I told andreww that yesterday.)
PDT is not sympathetic to this name change at this point in the delivery cycle. We're moving this down to P3 (generic standards).
I use the P-field for sorting my bugs. Leaving PDTP3 as indicator of PDT's opinion. This is P1 because it's the bug that is first on my list right now.
Hmmm. Consider that I have at least 2 crasher bugs related to the fact that the "behavior" property clashes with Microsoft's usage of the same property to load HTCs. Changing this name to -moz-binding would be a very good thing, and ensures that we don't end up crashing on any page that happens to use HTCs.
Marking crash per hyatt's comments. The last think in the world we want is for Gecko to be crashing on proprietary MS markup extensions as there's an increasing amount of that stuff floating around the web. That makes this issue now a PDT P1 in its own right.
The above list was more than just properties. The property renaming is done except for the exceptions in the next two paragraphs, and the other work still all needs to be done. Regarding counter-increment and counter-reset, I can't find *any* implementation of these properties, so I don't see a need to rename them. I also see parser errors where they're used in html.css. What does removing them from html.css do? (I strongly suspect nothing.) We also need to fix the DOM code not to work with the properties that we have disabled via the CSS parser, or to work with them using their -moz- names. This may mean just changing an idl file and regenerating code, but I'm not sure. See http://lxr.mozilla.org/seamonkey/find?string=CSS2Properties I'm running out of time to work on things this summer and I really need to finish up some other things too. There's a chance I may still be able to help with this, but I'm really not sure how much time I'll have next week. So, reassigning to pierre and clearing nsbeta3+ so he and his manager can reevaluate it according to their priorities.
I have DOM changes in my tree to handle mozBinding and mozOpacity rather than behavior and opacity. There's still the issue of how to handle outline, but that should be a separate bug.
Adding nsbeta3+ to status whiteboard.
Pierre: Give me a shout when you've finished this so that I can finish off the ua.css changes, please - thanks. If you can do it early this week I would be very thankful... ;-) If you think it is unlikely that you'll get to this before Thursday, then drop me a note so that I just do all the other changes and then let you fix them up later. (if that makes sense)
Resetting to P3. We are trusting that Hyatt will be fixing his other crasher bugs.
The DOM changes to CSS2Properties are in - so now we have MozBinding and MozOpacity instead of behavior and opacity (and they should work again).
*** Bug 52623 has been marked as a duplicate of this bug. ***
cc:ing mgalli as FYI as he does all kinds of things with opacity, including through the DOM, and will want to know.
Changing to nsbeta3-.
The whole point of this bug is that what we can't control is whether future CSS specs use properties that are currently extensions. We're renaming to -moz- to avoid forward-compatibility problems. If authors use something not defined by a current CSS spec, a future CSS spec could define it to mean something other than what the author meant. However, the property renaming is done -- that's not the issue anymore. The remaining stuff is all the other renaming (values, units, pseudos, &c.).
Is the renaming done for (a) static CSS markup, (b) JS DOM access through DOM2 CSS, and (c) XUL DOM CSS properties?
The property renaming is done for all 3. It is done in the CSS parser and the CSS2Properties interface. I think all other DOM use goes through the CSS parser. Perhaps it should be tested, though...
Note that the biggest problem at the moment is with our pseudo-classes, which is what the bug was originally filed about. ;-) In particular it is very likely that :table, :table-row and so on will eventually be added to CSS, and it is very UNLIKELY that the meaning will be the same. This is a major problem for future standards compliant authors.
Is there a particular technical reason that we were able to fix this for the CSS properties but not for the pseudo-classes? I'm perplexed that we fixed one but not the other. If the problem truly only remains for the pseudo classes, someone please rename the Summary to reflect this. If the pseudo-classes are still mis-named at this point, then I'm afraid we have to advise content developers to use my proposed workaround. Yes, it's ugly. Yes, David, in an ideal world content developers wouldn't have to do such things. But my job is about delivering the best product we can given the available time and resources in the real world and then helping developers write their code to work as well as possible on it. ;->
The only "workaround" available to content authors is *not to use* our extensions that have not been renamed. However, this is a forward-compatibility bug. The real problem that this bug causes is not now, it's later, after we fix this bug or implement these values or pseudo-classes differently according to later CSS specs. Proposing workarounds doesn't make sense for forward-compatibility bugs. The reason the properties were done and not the others is that they are in separate parts of the code, and I only had time to do one area.
David, if you have looked into it... - Is it sufficient to rename the pseudos in html.css and nsHTMLAtomList.h (+ cpp files)? Or do you know if they are used in other css files? - Which pseudos from nsHTMLAtomList.h have to be renamed? All?
All I know is what's in http://bugzilla.mozilla.org/showattachment.cgi?attach_id=14203 nsHTMLAtomList.h looks easy to do, because the atom name doesn't depend on the string, so we don't need to change code (although we probably should at some point to clean it up). My memory was that some of the others weren't so easy. I haven't looked to see what CSS files use which pseudos, but LXR is good for that.
Adding rtm+. This is low risk and necessary for standards compliance.
Marking rtm- due to the existence of potential workarounds and lack of fix.
Sometimes the PDT team leaves me speachless. I wasn't allowed to work on this bug until it was approved for a fix, and now that it is approved, it is immediately denied because there isn't a fix. Are you telling me that I should have found a fix between Friday afternoon and Monday morning? There isn't a workaround for this bug. Once we pollute the Net, it's very very hard to clean up. Putting back to [rtm+] for reconsideration.
Back to [rtm need info] per the new policy ("When the engineer and manager agree to work on the bug, the bug should be [rtm need info] and when the reviews are all done, the bug should be [rtm+].")
Although, thinking about it... I don't want to work on something that is may be refused later because of some misconceptions, so back to rtm+. Michael, why do you believe that we have some "potential workarounds"? Then if this bug has a reasonnable chance of being approved, please put [rtm need info], otherwise just [rtm-] again. Thanks.
Pierre, if you don't want to work on the fix just mark it rtm-. Otherwise, please provide the patch with review and super review for consideration.
Block moved M18 bugs to mozilla0.9.1
Changing [rtm need info] to [rtm-] per last week's discussion with PDT. Unfortunately, this didn't make RTM. Developers should consider these property names deprecated from the outset and subject to change and avoid building in dependencies on their use.
For the record: As mentioned previously, convincing developers not to use our extensions has nothing to do with this bug. The problem will occur when CSS _introduces_ a new feature that happens to have the same name as ours. Authors will then be unable to use the new features until N6 is no longer in the market. (e.g., if CSS3 introduces :canvas or :viewport, then N6 will choke on them.)
Don't you realize that this bug is going to make the work of thousands of web developers a nightmare??? I'm sure they will remember you and your decision when they try to make their pages work with NS6.(I worked with many web developers, and what they say when they have this kind of problems isn't funny) I really don't care about NS, but what really scares me is that they are going to blame mozilla :( And that when mozilla 1.0 ships I'll be unable of use standards without breaking netscape users. But if you care about NS, I think the anger of web developers isn't going to do any good to it. To make clear Hixie comment: This is going to make impossible the adoption of new standards until no one uses NS6!! Do you really want that people use NS6? Or you only want to ship? My only hope is that no one use NS6, it's really sad to say that about the first product based on mozilla :( P.S.: And if any one have some doubts: THERE IS NO WORKAROUND!! P.P.S.: Sorry if I'm too rough, but it is really frustrating to see years of hard work of many great hackers in standards compliance lost for this mistake :( Hixie: Don't get discouraged. Keep the good work, you rock! and Mozilla 1.0 is going to rock too ;)
Ian: You overlooked an important point. The point you made (about Mozilla having proprietary behaviors associated with these property names potentially discouraging adoption of the properties if the W3C adds the same property names to a a future spec version with different behaviors) is valid. *However*, it is *also* important for Netscape to document that these names and the behaviors associated with them are subject to change so that developers don't build in dependencies on the *proprietary* behaviors of these names in the code they create in the meantime, or so that if they do, they compartmentalize those dependencies for easy future upgrading. *That* is what I was referring to. Both of you: (1) This is an open source project. No one else stepped up to lend a hand on this bug, so don't slam Netscape only. (2) WebStandards.org itself has said that what Netscape can do most to help web standards is to ship Netscape 6 ASAP. The only way to do that is to close down development. The only way to do that is to not fix every outstanding bug. (3) You're overestimating the impact of this bug. First of all, it will be some time before the W3C releases new versions of specs, and there's no guarantee that all or any of these names will be used, and there's no guarantee that the property names will overlap, or if they do, there's no guarantee that the behaviors may not as well. Second, when a new W3C spec is released, it will in all likelihood be Netscape and mozilla.org that are implementing it first because we are implementing more web standards more deeply than anyone else. Third, we may get this fixed as soon as the first point release of N6. Fourth, there *are* workarounds (such as sniffing the browser version and then dynamically document.write()-ing out a link to one style sheet or another), and we can warn developers to use these approaches in the first place as a preventive measure if they use these properties so that these properties are only seen by N6 (not other browsers) in the first place. (4) Keep this bug in perspective. Where is the support of MS IE 5.5 for CSS1? What about the support of IE Mac 5 or Opera 4 for the DOM? We're implementing more web standards, more deeply, more consistently, across more platforms, simultaneously than any other browser engine available. If you're going to slam Netscape or anyone else about this particular bug, make sure to slam the other vendors proportionately for all the 1000+ cases where we get it right and they haven't even bothered to try.
Eric: What you say is correct. I just wanted to make sure that the facts were clear (hence my prefix "for the record"). For accuracy's sake: (1) is not a valid point, since the non-Netscape contributors to this project are aiming at Mozilla 1.0 or their own organisation's first release, and should not have to do things in time for *Netscape* deadlines. It is Netscape that is releasing this product, the responsability for its bugs lies clearly on Netscape (since, AOL and marketshare issues aside, Netscape could theoretically delay ship for this fix). (2) The last "word from the WaSP" was a farce, and has no respect in the standards-support community. IMHO the WaSP no longer speaks for that community, or if it does then it is fragmented. (3.1) This bug does indeed only have *potential* impact. However it is an impact that has been felt before, e.g. with IE's total mess-up with positioning (a mess-up that has caused a great deal of time to be wasted in the CSS community). I was hoping to avoid that as much as possible. Thankfully a more recent development (changing pseudo-elements to using two colons as a prefix) _may_ mitigate most of the issues here. (3.2) We can but do our best! :-) (3.3) That would be great. (3.4) Yes, this is the only workaround; a poor workaround indeed. I know that you are very aware of the awful problems we are having evangelising the fixing of all the existing browsing sniffing code. (4) I'll remind you that as far back as September 1998 I have been slamming Microsoft, Opera, and Microsoft again for their failures. Whew. Let's get this baby out of the door ASAP so we can get back to fixing these bugs! :-)
It seems unclear to me whether this bug requires either of a "developer" or "user" release note. If anyone feels it does, can they please draft one and then nominate with the relnote-user or relnote-rtm strings in the Status Whiteboard. Thanks :-) Gerv
Per previous comments, this cannot be release noted. Removing relnote keyword.
The XUL bindings have changed. It doesn't crash anymore, afaik. Removed the 'crash' keyword.
Time to nominate this bug again! :-)
Is the attached list still up to date? I'd be willing to give this a try (can't promise that I succeed though) but it would be nice to get some help with finding everything that needs changing. Also, what is the plan wrt the properties that wasn't fixed for NS6, is it ok to change them for mozilla1.0 anyway?
Moving to m1.0.
Hixie: can you help Jonas out with a bit of info here? Gerv
Jonas: You can assume the list is up to date. It might be slightly inaccurate in places, but that shouldn't be a big issue (dbaron?). Yes, it's safe to change things that were in Netscape 6.0 for Mozilla 1.0, since all the properties that people are likely to use (opacity, border-radius, float- edge) already have the -moz- prefix.
Moving to Mozilla1.1. Engineers are overloaded with higher priority bugs.
This really should be fixed by 1.0. It's basically an API, and we want to freeze APIs for 1.0, right?
Bulk moving from Moz1.1 to future-P1. I will pull from this list when scheduling work post Mozilla1.0.
Hixie: Here's an update on info I can find, and implementation notes for fixing this bug. Please could you cast your eye over it and tell me if I've got something wrong? Anything with " - none" beside it means that a grep of all the .css files in the tree failed to find a use of that word. " - unable to grep" means the word is so common in other contexts that it's very hard to see if the word is used by grepping. <snip> * new units to -moz- ch I can't find a use of these units, but it's hard to grep for. What are they? * new values to -moz- These names are changed in the file: http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsCSSKeywordList.h copy - none (I don't think) alias - none context-menu - none cell grab grabbing - none spinning - none count-up - none count-down - none count-up-down - none window  document  - none workspace  - none desktop  - none info  - unable to grep dialog  - unable to grep button  - unable to grep pull-down-menu  - none list  - unable to grep field  - unable to grep menu  - unable to grep Affected files: ./themes/classic/navigator/navigator.css ... * unknown values  - not sure about these! ignore - property of -moz-user-focus, -moz-outliner-image noshade - none paragraph - none start - property of -moz-box-align, -moz-box-pack, -text-align. * properties to -moz- http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsCSSPropList.h background-x-position is now -x-background-x-position background-y-position is now -x-background-y-position behavior is now -moz-binding border-x-spacing is now -x-border-x-spacing border-y-spacing is now -x-border-y-spacing box-sizing is now -moz-box-sizing clip-bottom is now -x-clip-bottom clip-left is now -x-clip-left clip-right is now -x-clip-right clip-top is now -x-clip-top counter-increment unused? Only used in tests counter-reset Used in html.css float-edge is now -moz-float-edge key-equivalent is now -moz-key-equivalent opacity is now -moz-opacity outline-color is now -moz-outline-color outline-style is now -moz-outline-style outline-width is now -moz-outline-width resizer is now -moz-resizer size-height is now -x-size-height size-width is now -x-size-width text-shadow-color is now -x-text-shadow-color text-shadow-radius is now -x-text-shadow-radius text-shadow-x is now -x-text-shadow-x text-shadow-y is now -x-text-shadow-y text-transform user-focus is now -moz-user-focus user-input is now -moz-user-input user-modify is now -moz-user-modify user-select is now -moz-user-select All of the renamed ones have a // XXX bug 3935 comment beside them. Does that mean that this bug fixed them, or there is still more to do? For the next three categories, I assume that what is required is to change the name in the .h file, and then change all the css files which reference that pseudo. Is that right? * nsLayoutAtomList pseudos to -moz- These names are changed in the file: http://lxr.mozilla.org/mozilla/source/content/shared/public/nsLayoutAtomList.h :canvas :scrolled-content :viewport :viewport-scroll Affected files: ./layout/html/document/src/html.css ./layout/html/document/src/forms.css * nsHTMLAtomList pseudos to -moz- These names are changed in the file: http://lxr.mozilla.org/mozilla/source/content/shared/public/nsHTMLAtomList.h :button-content :cell-content :body-column - none :fieldset-content :first-letter - none :first-line :frameset-blank - none :hframeset-border - none :ib-pseudo - none :label-content - none :legend-content - none :placeholder-frame - none :table :table-cell :table-column-group :table-column :table-outer :table-row-group :table-row :vframeset-border - none :wrapped-frame Affected files: ./layout/html/document/src/forms.css ./layout/html/document/src/html.css ./layout/html/forms/resources/content/xbl-forms.css ./content/xml/tests/books/common.css * nsCSSAtomList pseudos to -moz- These names are changed in the file: http://lxr.mozilla.org/mozilla/source/content/shared/public/nsCSSAtomList.h :checked :disabled - none :drag-over - none :drag - none :enabled - none :menu - none :root - none :selection - none Affected files: ./layout/html/document/src/forms.css  - need to be split up; these are the values for 'font'  - only do these if those marked  are done  - needs to be split up, this is the 'display' value not the others  - I cannot find ANYWHERE where these are used So, in summary: - I think I know how to fix the last three categories (the pseudos), and it's not hard. - All the properties apart from text-transform and maybe counter-* seem to be fixed already (and that text-transform is only used in test cases.) - A lot of the values don't seem to be used any more. I can fix some of them; tracking down others will be quite a bit of work because the words are common - I have no idea about the "ch" unit. Please advise :-) Gerv
The nsCSSKeywordList.h change omits inline-block ==> -moz-inline-block (see also bug 118711). Taking this bug and targetting 1.0, although Gerv's help is welcome.
Gerv: I don't see anything obviously wrong, but it's been a while since I have rummaged in the style system.
Here's a question: In http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsCSSPropList.h there are lines like: CSS_PROP(-moz-appearance, appearance, REFLOW) The comment says that: Entries are in the form: (name,id,effect). The second param should be the same as the first with "-" replaced with "_". This is not the case for a lot of the "-moz-" properties - the second param is missing the -moz-. Checking nsCSSPropList.cpp shows that we don't actually use the second param there - so does this matter at all? I have a patch for the last three things of the five (the pseudos), but I need more guidance on the first few from hixie or dbaron. Gerv
The second parameter is appended to eCSSProperty_ in nsCSSProps.h, and that enum value is used in the code. The name that is parsed is the first parameter. It's nice if they're consistent, but when I did the first patch for this bug I didn't want to make massive changes all over the code.
Created attachment 77540 [details] [diff] [review] Patch v.1 - fixes all pseudos This patch fixes all the pseudos (the bottom half of my list.) It's a start. Looking for r=dbaron or others. Gerv
Comment on attachment 77540 [details] [diff] [review] Patch v.1 - fixes all pseudos Most (maybe all) of the ones you changed in nsCSSAtomList.h are in the CSS3 selectors draft, which is in CR, so we should be implementing it without -moz-. :first-letter and :first-line are in CSS1, so they shouldn't be changed.
Regarding comment 79, I agree that we should change the counter-* properties, but I don't see the reason for changing text-transform. I'd need to look through the nsCSSKeywordList list more closely and check the spec, but I suspect some of those keywords may be real. (It would be useful to see a list of the properties for which they are accepted.) There may also be some that need to be split between a -moz- value and a non -moz- value (for different properties). And noshade and paragraph should just be removed from nsCSSKeywordList.h.
dbaron and others: I'm happy to fix this bug as soon as someone with knowledge of CSS drafts and Mozilla internals can tell me which ones we should be prefixing. If a definitive list can be defined, I'll produce the patch. Also, if anyone can help with the questions I had in comment 79 about the first few items in the current list ("ch" and the values), I'd be grateful for guidance there. Gerv
Created attachment 78781 [details] [diff] [review] Patch v.2 This patch tries to address all the comments made. It doesn't fix all the issues in this bug because exactly what to do has not been defined, but it's progress. I've compiled it, and tested web forms and other things which might be affected. I'd like to get this in for RC1 - that would mean it would need review over the weekend, probably. Gerv
Comment on attachment 78781 [details] [diff] [review] Patch v.2 At a quick glance, this patch is missing the nsCSSPropList.h changes.
is there another bug about converting all of xul everywhere (moz and commercial) to these new names? (Find and replace type stuff?)
Created attachment 78859 [details] [diff] [review] Patch v.3 Added fixes for counter- properties in nsCSSPropList.h, which (as I understand the comments) are the only changes required for that file. Is this patch now self-consistent? Gerv
</me has sinking feeling> andrew: I've searched all .css files in the tree for use of the properties I'm changing; are there other places I should be searching? Gerv
Comment on attachment 78859 [details] [diff] [review] Patch v.3 This patch looks good. I'll try to check later today that there aren't any other property or value changes that you need to make and go through in a little more detail. How did you search CSS files in the tree to decide what to change? The changes you found look like about what I would expect, since I wouldn't expect chrome CSS to be using any of these properties or pseudo-elements.
I searched all the CSS files by doing a "find" for them, then making a batch job which called "grep" with a very long list of arguments and one parameter :-) Gerv
Comment on attachment 78859 [details] [diff] [review] Patch v.3 The following can just be removed instead of renamed, since no code uses them, so there's no point wasting the memory for unused atoms: menuPseudo ibPseudo columnPseudo labelContentPseudo legendContentPseudo Could you mark these two changed lines: -CSS_PROP(counter-increment, counter_increment, REFLOW) -CSS_PROP(counter-reset, counter_reset, REFLOW) +CSS_PROP(-moz-counter-increment, counter_increment, REFLOW) +CSS_PROP(-moz-counter-reset, counter_reset, REFLOW) with "// XXX bug 48973" at the end, to be consistent with the key at the top, unless you want to change the key at the top to something more sensible (e.g., by filing a new bug about the 48973 issues). With those two minor changes, r=dbaron. (There still needs to be another patch for nsCSSValueList.h, right?)
*** Bug 136962 has been marked as a duplicate of this bug. ***
selectionPseudo and dragPseudo can also just be removed. (selectionPseudo is actually part of CSS3, but I don't forsee implementation in the near future, and the one place it's used in the current code just slows things down, and would only make sense if it were a pseudo-class rather than a pseudo-element anyway (see bug 128743).
Created attachment 79193 [details] [diff] [review] Patch v.4 Patch with changes requested by dbaron. > (There still needs to be another patch for nsCSSValueList.h, right?) Er... I can't find a file of that name, nor can I work out what you might have been referring to. Gerv
I meant nsCSSKeywordList.h. And it probably should be a separate patch -- it will may be more complicated than this one. And I can tell you didn't compile that patch yet. You're missing changes to nsCSSParser.cpp and nsCSSStyleSheet.cpp associated with comment 98. (You can just remove the relevant lines, they don't really do anything.)
(The nsCSSKeywordList.h changes also may be trivial. I think it just would take a while to figure out. I'm willing to do it.)
Created attachment 79307 [details] [diff] [review] Patch v.5 This one compiles :-) Gerv
Comment on attachment 79307 [details] [diff] [review] Patch v.5 r=dbaron
Comment on attachment 79307 [details] [diff] [review] Patch v.5 sr=jst
Comment on attachment 79307 [details] [diff] [review] Patch v.5 a=asa (on behalf of drivers) for checkin to the 1.0 branch
Patch v.5 checked in on branch and trunk. This bug remains open for the other work. Gerv
I made a few decisions that probably need to be discussed about what should be disabled and what shouldn't. (For example, I didn't disable 'text-align: start', because I think that CSS3 module is stable. I did disable 'inline-block' and 'inline-table'.) Someone will probably have to run the tool in bug 139943 over the commercial tree with this patch installed to search for potential changes that need to happen there.
Created attachment 81057 [details] [diff] [review] patch for nsCSSKeywordList changes, v. 2 I missed two keywords that could be completely removed since they're unused.
r=hixie. Looks good.
Created attachment 81089 [details] [diff] [review] patch for nsCSSKeywordList changes, v. 3 This patch removes 'display: menu', 'font: theme', and 'color: theme' completely, since hyatt says they're not used anymore. There was some code associated with copying around the values of the latter two (but not using it) that is also removed.
Comment on attachment 81089 [details] [diff] [review] patch for nsCSSKeywordList changes, v. 3 r=hixie
Comment on attachment 81089 [details] [diff] [review] patch for nsCSSKeywordList changes, v. 3 sr=waterson
Comment on attachment 81089 [details] [diff] [review] patch for nsCSSKeywordList changes, v. 3 email@example.com; please check into branch and trunk
Checked in to trunk 2002-04-30 17:16/17 PDT.
Fix checked in to MOZILLA_1_0_0_BRANCH, 2002-04-30 20:55 PDT. Since we're currently in the state we should be in, or at least pretty close, I'm marking this bug fixed. I filed bug 141397 for the long-term maintainance of this state, with the intention that it be revisited every few milestones.
I almost forgot: many thanks to Gerv and Hixie for considerable help in fixing this bug.
Verified on trunk and branch by checking the following files have been updated to use sanitised names: mozilla/layout/html/document/src/html.css mozilla/content/shared/public/nsCSSKeywordList.h mozilla/content/shared/public/nsCSSAtomList.h mozilla/content/shared/public/nsLayoutAtomList.h mozilla/content/shared/public/nsHTMLAtomList.h ...as well as various others.
Hmmm. We managed to miss :first-node and :last-node.
That was intentional.