From 2a022d4e34af24d64743f268382863dd246df017 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 29 Oct 2011 03:04:20 -0400 Subject: [PATCH] Bug 608756. Cache display structs in the ruletree even for floated and positioned elements. r=dbaron The basic idea is that mOriginalDisplay and mOriginalFloats are kept synchronized with mDisplay and mFloats unless the latter are changed due to position:absolute/fixed (for both) or float:left/right (for display). When initializing an nsStyleDisplay from a start struct, we restore the values from mOriginalDisplay/Floats to get correct behavior. --- layout/reftests/bugs/608756-1-ref.html | 4 +++ layout/reftests/bugs/608756-1a.html | 10 ++++++ layout/reftests/bugs/608756-1b.html | 10 ++++++ layout/reftests/bugs/608756-2-ref.html | 9 ++++++ layout/reftests/bugs/608756-2.html | 10 ++++++ layout/reftests/bugs/reftest.list | 3 ++ layout/style/nsRuleNode.cpp | 42 +++++++++++++++++--------- layout/style/nsStyleContext.cpp | 11 +++++-- layout/style/nsStyleStruct.cpp | 4 ++- layout/style/nsStyleStruct.h | 4 +++ 10 files changed, 89 insertions(+), 18 deletions(-) create mode 100644 layout/reftests/bugs/608756-1-ref.html create mode 100644 layout/reftests/bugs/608756-1a.html create mode 100644 layout/reftests/bugs/608756-1b.html create mode 100644 layout/reftests/bugs/608756-2-ref.html create mode 100644 layout/reftests/bugs/608756-2.html diff --git a/layout/reftests/bugs/608756-1-ref.html b/layout/reftests/bugs/608756-1-ref.html new file mode 100644 index 00000000000..d62c43d9edc --- /dev/null +++ b/layout/reftests/bugs/608756-1-ref.html @@ -0,0 +1,4 @@ + + +Test
Test + diff --git a/layout/reftests/bugs/608756-1a.html b/layout/reftests/bugs/608756-1a.html new file mode 100644 index 00000000000..aacca22b926 --- /dev/null +++ b/layout/reftests/bugs/608756-1a.html @@ -0,0 +1,10 @@ + + + + + + +Test
Test + diff --git a/layout/reftests/bugs/608756-1b.html b/layout/reftests/bugs/608756-1b.html new file mode 100644 index 00000000000..98ffe0a8092 --- /dev/null +++ b/layout/reftests/bugs/608756-1b.html @@ -0,0 +1,10 @@ + + + + + + +Test
Test + diff --git a/layout/reftests/bugs/608756-2-ref.html b/layout/reftests/bugs/608756-2-ref.html new file mode 100644 index 00000000000..c68452f5221 --- /dev/null +++ b/layout/reftests/bugs/608756-2-ref.html @@ -0,0 +1,9 @@ + + + + + +Test
Test + diff --git a/layout/reftests/bugs/608756-2.html b/layout/reftests/bugs/608756-2.html new file mode 100644 index 00000000000..558462b9d89 --- /dev/null +++ b/layout/reftests/bugs/608756-2.html @@ -0,0 +1,10 @@ + + + + + + +Test
Test + diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index 462e9a12cfe..9f92c3f2e42 100644 --- a/layout/reftests/bugs/reftest.list +++ b/layout/reftests/bugs/reftest.list @@ -1599,6 +1599,9 @@ fails-if(!haveTestPlugin) == 599476.html 599476-ref.html == 604737.html 604737-ref.html == 605138-1.html 605138-1-ref.html == 605157-1.xhtml 605157-1-ref.xhtml +== 608756-1a.html 608756-1-ref.html +== 608756-1b.html 608756-1-ref.html +== 608756-2.html 608756-2-ref.html == 609272-1.html 609272-1-ref.html == 608636-1.html 608636-1-ref.html needs-focus == 613433-1.html 613433-1-ref.html diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index 6f2d45ebf21..27336bf8ea5 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -3875,6 +3875,14 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct, { COMPUTE_START_RESET(Display, (), display, parentDisplay) + // We may have ended up with aStartStruct's values of mDisplay and + // mFloats, but those may not be correct if our style data overrides + // its position or float properties. Reset to mOriginalDisplay and + // mOriginalFloats; it if turns out we still need the display/floats + // adjustments we'll do them below. + display->mDisplay = display->mOriginalDisplay; + display->mFloats = display->mOriginalFloats; + // Each property's index in this array must match its index in the // const array |transitionPropInfo| above. TransitionPropData transitionPropData[4]; @@ -4281,6 +4289,10 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct, SetDiscrete(*aRuleData->ValueForDisplay(), display->mDisplay, canStoreInRuleTree, SETDSC_ENUMERATED, parentDisplay->mDisplay, NS_STYLE_DISPLAY_INLINE, 0, 0, 0, 0); + // Backup original display value for calculation of a hypothetical + // box (CSS2 10.6.4/10.6.5), in addition to getting our style data right later. + // See nsHTMLReflowState::CalculateHypotheticalBox + display->mOriginalDisplay = display->mDisplay; // appearance: enum, inherit, initial SetDiscrete(*aRuleData->ValueForAppearance(), @@ -4358,6 +4370,8 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct, display->mFloats, canStoreInRuleTree, SETDSC_ENUMERATED, parentDisplay->mFloats, NS_STYLE_FLOAT_NONE, 0, 0, 0, 0); + // Save mFloats in mOriginalFloats in case we need it later + display->mOriginalFloats = display->mFloats; // overflow-x: enum, inherit, initial SetDiscrete(*aRuleData->ValueForOverflowX(), @@ -4483,7 +4497,8 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct, if (nsCSSPseudoElements::firstLetter == aContext->GetPseudo()) { // a non-floating first-letter must be inline // XXX this fix can go away once bug 103189 is fixed correctly - display->mDisplay = NS_STYLE_DISPLAY_INLINE; + // Note that we reset mOriginalDisplay to enforce the invariant that it equals mDisplay if we're not positioned or floating. + display->mOriginalDisplay = display->mDisplay = NS_STYLE_DISPLAY_INLINE; // We can't cache the data in the rule tree since if a more specific // rule has 'float: left' we'll end up with the wrong 'display' @@ -4494,28 +4509,25 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct, if (display->IsAbsolutelyPositioned()) { // 1) if position is 'absolute' or 'fixed' then display must be // block-level and float must be 'none' - - // Backup original display value for calculation of a hypothetical - // box (CSS2 10.6.4/10.6.5). - // See nsHTMLReflowState::CalculateHypotheticalBox - display->mOriginalDisplay = display->mDisplay; EnsureBlockDisplay(display->mDisplay); display->mFloats = NS_STYLE_FLOAT_NONE; - // We can't cache the data in the rule tree since if a more specific - // rule has 'position: static' we'll end up with problems with the - // 'display' and 'float' properties. - canStoreInRuleTree = false; + // Note that it's OK to cache this struct in the ruletree + // because it's fine as-is for any style context that points to + // it directly, and any use of it as aStartStruct (e.g. if a + // more specific rule sets "position: static") will use + // mOriginalDisplay and mOriginalFloats, which we have carefully + // not changed. } else if (display->mFloats != NS_STYLE_FLOAT_NONE) { // 2) if float is not none, and display is not none, then we must // set a block-level 'display' type per CSS2.1 section 9.7. - EnsureBlockDisplay(display->mDisplay); - // We can't cache the data in the rule tree since if a more specific - // rule has 'float: none' we'll end up with the wrong 'display' - // property. - canStoreInRuleTree = false; + // Note that it's OK to cache this struct in the ruletree + // because it's fine as-is for any style context that points to + // it directly, and any use of it as aStartStruct (e.g. if a + // more specific rule sets "float: none") will use + // mOriginalDisplay, which we have carefully not changed. } } diff --git a/layout/style/nsStyleContext.cpp b/layout/style/nsStyleContext.cpp index 80ae1927e9a..1cb86e6213e 100644 --- a/layout/style/nsStyleContext.cpp +++ b/layout/style/nsStyleContext.cpp @@ -374,10 +374,17 @@ nsStyleContext::ApplyStyleFixups(nsPresContext* aPresContext) disp->mDisplay != NS_STYLE_DISPLAY_TABLE) { nsStyleDisplay *mutable_display = static_cast (GetUniqueStyleData(eStyleStruct_Display)); + // If we're in this code, then mOriginalDisplay doesn't matter + // for purposes of the cascade (because this nsStyleDisplay + // isn't living in the ruletree anyway), and for determining + // hypothetical boxes it's better to have mOriginalDisplay + // matching mDisplay here. if (mutable_display->mDisplay == NS_STYLE_DISPLAY_INLINE_TABLE) - mutable_display->mDisplay = NS_STYLE_DISPLAY_TABLE; + mutable_display->mOriginalDisplay = mutable_display->mDisplay = + NS_STYLE_DISPLAY_TABLE; else - mutable_display->mDisplay = NS_STYLE_DISPLAY_BLOCK; + mutable_display->mOriginalDisplay = mutable_display->mDisplay = + NS_STYLE_DISPLAY_BLOCK; } } diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 20d3b23ec08..988070a0bd2 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -2080,9 +2080,10 @@ nsStyleDisplay::nsStyleDisplay() MOZ_COUNT_CTOR(nsStyleDisplay); mAppearance = NS_THEME_NONE; mDisplay = NS_STYLE_DISPLAY_INLINE; - mOriginalDisplay = NS_STYLE_DISPLAY_NONE; + mOriginalDisplay = mDisplay; mPosition = NS_STYLE_POSITION_STATIC; mFloats = NS_STYLE_FLOAT_NONE; + mOriginalFloats = mFloats; mBreakType = NS_STYLE_CLEAR_NONE; mBreakBefore = false; mBreakAfter = false; @@ -2146,6 +2147,7 @@ nsStyleDisplay::nsStyleDisplay(const nsStyleDisplay& aSource) mAppearance = aSource.mAppearance; mDisplay = aSource.mDisplay; mOriginalDisplay = aSource.mOriginalDisplay; + mOriginalFloats = aSource.mOriginalFloats; mBinding = aSource.mBinding; mPosition = aSource.mPosition; mFloats = aSource.mFloats; diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index db96bc65811..332b089d570 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -1539,9 +1539,13 @@ struct nsStyleDisplay { float mOpacity; // [reset] PRUint8 mDisplay; // [reset] see nsStyleConsts.h NS_STYLE_DISPLAY_* PRUint8 mOriginalDisplay; // [reset] saved mDisplay for position:absolute/fixed + // and float:left/right; otherwise equal + // to mDisplay PRUint8 mAppearance; // [reset] PRUint8 mPosition; // [reset] see nsStyleConsts.h PRUint8 mFloats; // [reset] see nsStyleConsts.h NS_STYLE_FLOAT_* + PRUint8 mOriginalFloats; // [reset] saved mFloats for position:absolute/fixed; + // otherwise equal to mFloats PRUint8 mBreakType; // [reset] see nsStyleConsts.h NS_STYLE_CLEAR_* bool mBreakBefore; // [reset] bool mBreakAfter; // [reset]