205790 - do inheritance without layout-dependent computations (CSS2.1 change) (remove eStyleUnit_Inherit and nsStyleCoord::SetInheritValue)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Hixie (not reading bugmail)]

According to CSS2.1 (a CR for which a fix for this bug should probably wait),
inheritance is no longer layout-dependent in any case.

This should make it possible to optimise our system and remove the need for an
inheritance value in the style contexts. (I sure hope what I'm saying is making
sense. Ahem.)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds great. This is a fantastic move and we should definitely take advantage
of it.

I assume it will still be true in CSS3?

Comment 2

[Hixie (not reading bugmail)]

I sure hope so.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm. What exactly has changed in CSS2.1? For example, given

<div style="position:absolute; top:0; left:0;">
  Hello Kitty
  <div style="position:absolute; top:100px; left:0; height:inherit; background:red">
    Hello Kitty
  </div>
</div>

CSS2.1 still seems to say that the red DIV gets the computed height of the outer
DIV, which requires layout.

Comment 4

[Hixie (not reading bugmail)]

The change I mentioned has not yet been included in any published version of
CSS2.1. The intention is that the inherited value in the case you give would be
'auto' (assuming that is the specified value on the outer DIV too, which it
would be in the absence of any other stylesheets).

Comment 5

[Hixie (not reading bugmail)]

This will affect bug 117887, bug 93853, and bug 45631.

Comment 6

[Hixie (not reading bugmail)]

*** Bug 226474 has been marked as a duplicate of this bug. ***

Comment 7

[Hixie (not reading bugmail)]

from dbaron's description of bug 226474:
> The changes in CSS 2.1 mean that we should be able to remove eStyleUnit_Inherit
> and nsStyleCoord::SetInheritValue.  This will let us simplify a good bit of 
> code...

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The patch I'm going to attach will, I think, make the iscoord_ parameter in the
CSSPropList macros unused.  However, I want to leave it since I'm going to need
something similar (although not exactly the same) for bug 147777.

I think this patch is low risk because it's removing very poorly tested code,
and code that should incompatible with what any other browser that actually
supports 'inherit' does.  I'd like to get it in now because I think it might
make bug 215857 easier to understand and/or fix.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -ub)

This patch uses diff -b (ignore changes in amount of whitespace).

If you really want to review my indentation changes, I can attach that version
as well...

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 136070 [details] [diff] [review]
patch (diff -ub)

I think I can trust you on indentation changes... ;)

r+sr=bzbarsky; I'm glad to see some of that code go....

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 136070 [details] [diff] [review]
patch (diff -ub)

See comment 8 about risk.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 136070 [details] [diff] [review]
patch (diff -ub)

a=asa (on behalf of drivers) for checkin to 1.6beta

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The regression tests that changed (ignoring the usual noise and the style data
mismatches that are expected) are:

table/bugs/bug32447.html (moved a little to the right)
table/bugs/bug34538.html (exact same test as previous)
table/bugs/bug89315.html (invisible bbox change, maybe noise)

So the only real change that I'm worried about is attachment 6700 [details].

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The change to attachment 6700 [details] is more compatible with IE/Windows, and I think
more correct.  (I think it's because the 'left: inherit', etc., on
::-moz-table-outer are now working as intended.)

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in, 2003-11-24 11:46 -0800.

Comment 16

[Boris Zbarsky [:bzbarsky]]

So... I wonder how this checkin affected bug 53663 and its dependencies.  See
bug 53663 comment 12.

Comment 17

[Hixie (not reading bugmail)]

Well, 'inherit' still isn't the specified value.

Comment 18

[Boris Zbarsky [:bzbarsky]]

What do you mean, Ian?

Comment 19

[Hixie (not reading bugmail)]

bz: The comment you linked to says we want to "inherit" the specified value. But
in CSS2.1, the 'inherit' keyword inherits the computed value, and even in CSS2.1
the computed value is not the specified value.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Actually, I think the computed and specified values are fully equivalent for the
purposes of that code.  The key here is that "inherit" takes place at style
resolution time instead of layout time, so "inherit" when a parent has "auto"
will become "auto" (which is the problem in bug 53663).

Comment 21

[Hixie (not reading bugmail)]

Well if that comment is wrong, then nevermind. :-)

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800)

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 167449 [details] [diff] [review]
remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800)

I don't think the bit in comment 8 about bug 147777 is true anymore.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 167449 [details] [diff] [review]
remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800)

r+sr=bzbarsky

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700)

I missed removing ", inherit" from a bunch of comments in nsStyleStruct.h

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 197373 [details] [diff] [review]
fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700)

r+sr=bzbarsky