Closed Bug 80887 Opened 24 years ago Closed 15 years ago

Add -moz-initial to CSS

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: hyatt, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

(Keywords: css-moz, css3)

Attachments

(1 file)

implement almost all remaining properties
32.01 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
As part of 76895, I am going to be adding a -moz-initial keyword to CSS. You can use this keyword to specify that - for a property that is normally inherited by default (e.g., color) - you should use the initial value that you'd get if you had no parent. This will allow me to eliminate all the special-case table quirks code and specify the quirks for tables in quirk.css. I will do this right after converting all 15 structs on the branch before I land. If anyone wants to help out by crafting the correct style rules in advance, that would be most appreciated.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
Blocks: 78695
Summary: Add -moz-initial to CSS → [CSS] Add -moz-initial to CSS
Keywords: css-moz
Be careful about how you parse this for properties that accept random unquoted strings, like 'font-family' or 'counter-reset'. It should only be understood if '-moz-initial' is the absolute only token in the value. Copying the way we do it for 'inherit' should be ok. Just saying that for completeness! :-) As far as what rules we should do... I'm not familiar with them, but something along the lines of: table { font: -moz-initial; font-family: inherit; } table, thead, tfoot, tbody, tr, td, th { background: inherit; } I don't know how you would do the resetting of 'color' (which has to be done for 'table', 'table :link', and 'table :visited'), since that has to reset to the value given on the <body> element's attributes. Are there any other table quirks?
I can leave the document color rule in C++. I was more concerned with the code in the RemapStyle method (which handles fonts).
As we discovered, you also need something along the lines of: table { text-align: -moz-initial; } In standard mode, we can't do that because it should only happen if the inherited text-align is one of -moz-right or -moz-center, not anything else, so we still need code for that (it's already there). We can do the mapping of the align attribute of <table> into CSS quite easily because there are no quirks there, right? (dbaron?) The mapping of inherited text-aligns of '-moz-right' and '-moz-center' (set by DIV align attribute rules) into values for float will also have to be done at the C++ level. dbaron: Do you have any test cases for these issues? They would be really useful here! :-)
The -moz-right/-moz-center stuff is really rather a mess, but that's at least partly because the required behavior is messy as well. The current solution is described in bug 37083 and bug 40038, both of which have various test cases (and the patch is on bug 40038). The particularly messy part is that I had to add a slight table inheritance quirk into standard mode (where the rest of the table inheritance quirks are, but shouldn't be, in nsStyleContext.cpp), to prevent alignment that comes from HTML from inheriting into tables. I imagine hyatt's going to have to redesign these quirks a bit for the stuff he's rewriting. I suspect many of the quirks can be done more elegantly than they are now, but I think some really do require really weird things. If you want me to describe the issues in those bugs more, just ask... (BTW, I don't think the mapping of align to float has to be done in C++. It's dealing with inherited text-align and align attributes on the parent of tables that's messy.)
>I imagine hyatt's going to have to redesign these quirks a bit for the stuff >he's rewriting. I suspect many of the quirks can be done more elegantly than >they are now, but I think some really do require really weird things. And I just read the first comment on this bug...
I may as well summarize how things work. This is basically from memory of work I did almost a year ago, so take with appropriate grains of salt... The ALIGN attribute on TABLE is simple. right and left map to float, center maps to auto margins. What is complicated is the ALIGN attribute on a DIV that contains a TABLE, or on table cell that contains a TABLE. (We treat it similarly for a P, but a P can't contain a TABLE.) These instances of the ALIGN attribute need to cause 3 things: 1. the equivalent of 'text-align' (left, center, right) for any line boxes within the ALIGN-ed block and its descendants 2. The creation of appropriate auto margins on the ALIGN-ed block and its descendants, except... 3. Both of these effects need to be undone within a table. (The auto margins do get applied to the table, though.) That is, inheritance has to be blocked. However, the CSS 'text-align' property has to inherit into tables in strict mode. (Is there a good reason not to do it in quirks mode? What is the extent of MSIE's table inheritance quirks?) Our current solution is to map this ALIGN attribute to -moz-right/-moz-center (I have a patch for -moz-left in my tree). We then: 1. treat -moz-right/-moz-center the same as right/center in the text alignment code. 2. allow it to override auto margins in the block reflow code. (nsHTMLReflowState.cpp, nsBlockReflowContext.cpp) 3. have a hack to extend the table inheritance quirk a tiny bit into strict mode, for -moz-center and -moz-right only. (nsStyleContext.cpp) I haven't looked into the table inheritance quirks in detail for over two years. At the time I think I was convinced that they could be handled in quirks.css. (See bug 1044 for the bug that was being discussed, although I always remember it as having much more discusion than is there. It was probably by email or on newsgroups.) I'd probably have some interesting comments about them if I had a chance to refresh my memory. How much of them are you rewriting and what's your current plan?
I have retained the hack that resets text-align on tables that inherit -moz- center from a parent <div> or <td>. I changed it so that text-align *is* inherited into tables even in quirks mode, and that for both quirks and strict mode I use the hack. This seems to work ok. Any reason why in quirks mode I'd have to cut off text- align inheritance completely (even for normal CSS values and not for hacky -moz- XXX values)?
Have a look at what IE's table-inheritance quirks are. IIRC, when I looked at table inheritance quirks 2 years ago, we did them on practically all inheritable properties when we really only needed them on a handful to be compatible with the minimum of what IE and 4.x did, which is what's needed for the web. So I suspect it's fine. (Maybe I'll figure out what IE and 4.x do again this weekend...)
IE has much less quirky behaviour in tables (none for backrounds AFAIK) then NS4.x. However I suspect that netscape want to go with 4.x behaviour. Personally I like to go with as few quirks as possible, the fewer the better.
mozilla.org wants fewer quirks, too (and no quirk-mechanism generality overkill, if you know what I mean). Does netscape.com really care about those 4.x table quirks? Cc'ing bclary. /be
->moz1.0/helpwanted
Keywords: helpwanted
Target Milestone: mozilla0.9.2 → mozilla1.0
I don't think we want to hobble ourselve with what Navigator 4 did/does. Mozilla and Netscape 6 are completely different beasts than Navigator 4 and trying to emulate Navigator 4 quirks only makes life more complicated rather than simpler. IMHO, Mozilla has more in common with IE5 than it does with Navigator 4. Of course this is only my opinion, but I will run it by the people who pay me and see what they say. More at 11.
Target Milestone: mozilla1.0 → mozilla1.0.1
It should also be noted that most developers no longer take netscape 4.7 in to consideration. Those few larger companies that do, will probably add detection for this mozilla version and use IE pages instead. BTW: Is this bug dead?
No, this still needs to be fixed for many other properties.
OS: Windows 2000 → All
Hardware: PC → All
Summary: [CSS] Add -moz-initial to CSS → Add -moz-initial to CSS
So I'd like to finish this at some point. I'm tempted to remove the default values from the nsStyleStruct constructors and change the initial checks to be: if (...GetUnit() == eCSSUnit_Initial || (fillInitial && ...GetUnit() == eCSSUnit_Null)) Where fillInitial = !aStartStruct for the reset structs and is set to true only in the default constructor case for the inherited structs (where the test is more complicated than just !aStartStruct). Does this seem reasonable? (Does it seem like it will work?)
Assignee: hyatt → dbaron
Status: ASSIGNED → NEW
Keywords: helpwanted → css3
Target Milestone: mozilla1.0.1 → ---
That sounds like it should work, assuming SetDefaultOnRoot is also fixed to set the initial values. Also, that snippet of code sounds like it would do well as a macro... ;)
Assignee: dbaron → nobody
QA Contact: ian → style-system
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #273412 - Flags: superreview?(bzbarsky)
Attachment #273412 - Flags: review?(bzbarsky)
Comment on attachment 273412 [details] [diff] [review] implement almost all remaining properties >+ else if (eCSSUnit_Initial == displayData.mClip.mTop.GetUnit()) { >+ display->mClipFlags = NS_STYLE_CLIP_AUTO; >+ display->mClip.SetRect(0,0,0,0); Wouldn't SetRect(0, NS_MAXSIZE, NS_MAXSIZE, 0) better match what "clip: auto" does? I think we should do that here. > nsCSSValueList* list = > marginData.mBorderColors.*(nsCSSValueListRect::sides[side]); >+ // FIXME: Implement inherit and -moz-initial. Followup bug, and cite bug number in comment? > else if (eCSSUnit_Enumerated == marginData.mOutlineColor.GetUnit()) > outline->SetOutlineInitialColor(); >+ else if (eCSSUnit_Initial == marginData.mOutlineColor.GetUnit()) { >+ outline->SetOutlineInitialColor(); >+ } Why not combine those two clauses together? >@@ -4035,6 +4171,8 @@ nsRuleNode::ComputeQuotesData >+ // FIXME: Implement eCSSUnit_Initial (correctly, unlike nsStyleStruct), and >+ // remove the "initial" value from ua.css. Mention the bug filed on that in the comment? >@@ -4359,6 +4515,7 @@ nsRuleNode::ComputeSVGData >+ // FIXME This is broken when aStartStruct is non-null! > if (!svg->mStrokeDasharray) { Mention bug number in comment? r+sr=bzbarsky with those nits. I'd still like to get rid of the duplication we have with the struct constructors, though... I guess separate bug on that. :(
Attachment #273412 - Flags: superreview?(bzbarsky)
Attachment #273412 - Flags: superreview+
Attachment #273412 - Flags: review?(bzbarsky)
Attachment #273412 - Flags: review+
Blocks: 389014
(In reply to comment #18) > >+ else if (eCSSUnit_Initial == displayData.mClip.mTop.GetUnit()) { > >+ display->mClipFlags = NS_STYLE_CLIP_AUTO; > >+ display->mClip.SetRect(0,0,0,0); > > Wouldn't SetRect(0, NS_MAXSIZE, NS_MAXSIZE, 0) better match what "clip: auto" > does? I think we should do that here. I'm just matching what the nsStyleStruct constructors do. (It's annoying duplication that I'd like to get rid of when we make this more table-driven (bug 332335).) And what really matters is the mClipFlags; when that's NS_STYLE_CLIP_AUTO the mClip should be ignored.
Checked in to trunk; bug 389406 is still a dependency for fixing this.
Depends on: 389406
What about bug 389404?
Does a list of remaining properties to support exist? When having a bug-free implementation of (almost?) all properties, I guess the -moz prefix can be dropped? (which probably belongs in a bug of it's own)
It's now supported on all supported CSS properties. However, 'initial' is not in CR yet, so we should keep the prefix.
I guess that means this is fixed; remaining issues were fixed by bug 389406, bug 160403, bug 376075.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
I guess https://developer.mozilla.org/En/CSS/Initial needs an update, as it doesn't say that initial is now supported on all properties (that is supported in the browser at all).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: