Closed Bug 80887 Opened 23 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: