Last Comment Bug 205790 - do inheritance without layout-dependent computations (CSS2.1 change) (remove eStyleUnit_Inherit and nsStyleCoord::SetInheritValue)
: do inheritance without layout-dependent computations (CSS2.1 change) (remove ...
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.6beta
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
: Hixie (not reading bugmail)
Mentors:
: 226474 (view as bug list)
Depends on:
Blocks: 203448 53663 93865 208274
  Show dependency treegraph
 
Reported: 2003-05-15 04:13 PDT by Hixie (not reading bugmail)
Modified: 2007-12-10 03:38 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (diff -ub) (80.18 KB, patch)
2003-11-21 15:16 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
bzbarsky: superreview+
asa: approval1.6b+
Details | Diff | Review
remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800) (87.18 KB, patch)
2004-11-30 10:53 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Review
fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700) (5.48 KB, patch)
2005-09-25 17:21 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Review

Description Hixie (not reading bugmail) 2003-05-15 04:13:27 PDT
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) (Exited; email my personal email if necessary) 2003-05-15 06:30:57 PDT
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) 2003-05-15 06:35:14 PDT
I sure hope so.
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2003-05-15 08:55:30 PDT
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) 2003-05-15 09:19:25 PDT
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) 2003-06-17 04:59:33 PDT
This will affect bug 117887, bug 93853, and bug 45631.
Comment 6 Hixie (not reading bugmail) 2003-11-21 14:19:35 PST
*** Bug 226474 has been marked as a duplicate of this bug. ***
Comment 7 Hixie (not reading bugmail) 2003-11-21 14:24:00 PST
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-7 (review requests must explain patch) 2003-11-21 14:41:11 PST
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-7 (review requests must explain patch) 2003-11-21 15:16:49 PST
Created attachment 136070 [details] [diff] [review]
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 [:bz] (Out June 25-July 6) 2003-11-23 12:00:22 PST
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-7 (review requests must explain patch) 2003-11-23 16:34:33 PST
Comment on attachment 136070 [details] [diff] [review]
patch (diff -ub)

See comment 8 about risk.
Comment 12 Asa Dotzler [:asa] 2003-11-24 10:50:39 PST
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-7 (review requests must explain patch) 2003-11-24 11:33:24 PST
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-7 (review requests must explain patch) 2003-11-24 11:36:53 PST
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-7 (review requests must explain patch) 2003-11-24 11:49:46 PST
Fix checked in, 2003-11-24 11:46 -0800.
Comment 16 Boris Zbarsky [:bz] (Out June 25-July 6) 2003-11-24 12:56:39 PST
So... I wonder how this checkin affected bug 53663 and its dependencies.  See
bug 53663 comment 12.
Comment 17 Hixie (not reading bugmail) 2003-11-24 12:59:29 PST
Well, 'inherit' still isn't the specified value.
Comment 18 Boris Zbarsky [:bz] (Out June 25-July 6) 2003-11-24 13:15:44 PST
What do you mean, Ian?
Comment 19 Hixie (not reading bugmail) 2003-11-24 13:30:59 PST
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 [:bz] (Out June 25-July 6) 2003-11-24 13:38:28 PST
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) 2003-11-24 14:05:02 PST
Well if that comment is wrong, then nevermind. :-)
Comment 22 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-11-30 10:53:30 PST
Created attachment 167449 [details] [diff] [review]
remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800)
Comment 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-11-30 10:54:11 PST
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 [:bz] (Out June 25-July 6) 2004-11-30 11:06:11 PST
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-7 (review requests must explain patch) 2005-09-25 17:21:55 PDT
Created attachment 197373 [details] [diff] [review]
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 [:bz] (Out June 25-July 6) 2005-09-25 18:30:42 PDT
Comment on attachment 197373 [details] [diff] [review]
fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700)

r+sr=bzbarsky

Note You need to log in before you can comment on or make changes to this bug.