Closed Bug 205790 Opened 22 years ago Closed 21 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.6beta

People

(Reporter: ian, Assigned: dbaron)

References

Details

(Keywords: perf)

Attachments

(3 files)

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.)
Sounds great. This is a fantastic move and we should definitely take advantage of it. I assume it will still be true in CSS3?
I sure hope so.
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.
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).
Blocks: 203448
OS: Windows 2000 → All
Hardware: PC → All
This will affect bug 117887, bug 93853, and bug 45631.
Summary: Optimise inheritance to not require layout-dependent computations → Optimise inheritance to not require layout-dependent computations (change how inheriting computed values works for CSS2.1)
*** Bug 226474 has been marked as a duplicate of this bug. ***
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...
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.6beta
Summary: Optimise inheritance to not require layout-dependent computations (change how inheriting computed values works for CSS2.1) → do inheritance without layout-dependent computations (CSS2.1 change) (remove eStyleUnit_Inherit and nsStyleCoord::SetInheritValue)
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.
Attached patch patch (diff -ub)Splinter Review
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...
Attachment #136070 - Flags: superreview?(bz-vacation)
Attachment #136070 - Flags: review?(bz-vacation)
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....
Attachment #136070 - Flags: superreview?(bz-vacation)
Attachment #136070 - Flags: superreview+
Attachment #136070 - Flags: review?(bz-vacation)
Attachment #136070 - Flags: review+
Comment on attachment 136070 [details] [diff] [review] patch (diff -ub) See comment 8 about risk.
Attachment #136070 - Flags: approval1.6b?
Comment on attachment 136070 [details] [diff] [review] patch (diff -ub) a=asa (on behalf of drivers) for checkin to 1.6beta
Attachment #136070 - Flags: approval1.6b? → approval1.6b+
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].
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.)
Fix checked in, 2003-11-24 11:46 -0800.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
So... I wonder how this checkin affected bug 53663 and its dependencies. See bug 53663 comment 12.
Blocks: 53663
Well, 'inherit' still isn't the specified value.
What do you mean, Ian?
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.
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).
Well if that comment is wrong, then nevermind. :-)
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.
Attachment #167449 - Flags: superreview?(bzbarsky)
Attachment #167449 - Flags: review?(bzbarsky)
Comment on attachment 167449 [details] [diff] [review] remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800) r+sr=bzbarsky
Attachment #167449 - Flags: superreview?(bzbarsky)
Attachment #167449 - Flags: superreview+
Attachment #167449 - Flags: review?(bzbarsky)
Attachment #167449 - Flags: review+
Attachment #167449 - Attachment description: remove iscoord → remove iscoord (checked in to trunk, 2004-11-30 11:24 -0800)
I missed removing ", inherit" from a bunch of comments in nsStyleStruct.h
Attachment #197373 - Flags: superreview?(bzbarsky)
Attachment #197373 - Flags: review?(bzbarsky)
Comment on attachment 197373 [details] [diff] [review] fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700) r+sr=bzbarsky
Attachment #197373 - Flags: superreview?(bzbarsky)
Attachment #197373 - Flags: superreview+
Attachment #197373 - Flags: review?(bzbarsky)
Attachment #197373 - Flags: review+
Attachment #197373 - Attachment description: fix comments in nsStyleStruct.h → fix comments in nsStyleStruct.h (checked in, 2005-09-25 18:58 -0700)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: