Bug 655549 and bug 645768. Rejigger the quirk table color rule to work more reliably. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 01 Jun 2011 07:43:31 -0400
changeset 70450 9439ffe089a13715e7e9aa5ad1db5c324f3984d1
parent 70418 75907ce0bcbb84205e00d991fd6b38aa9a7de41a
child 70451 2d0b50884c7a4e861f36fc7a38c0845d78315d44
push id20324
push usermlamouri@mozilla.com
push dateThu, 02 Jun 2011 11:11:19 +0000
treeherdermozilla-central@1e3b54a01913 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs655549, 645768
milestone7.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 655549 and bug 645768. Rejigger the quirk table color rule to work more reliably. r=dbaron
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/base/nsStyleConsts.h
layout/reftests/bugs/645768-1-ref.html
layout/reftests/bugs/645768-1.html
layout/reftests/bugs/655549-1-ref.html
layout/reftests/bugs/655549-1.html
layout/reftests/bugs/reftest.list
layout/style/nsHTMLStyleSheet.cpp
layout/style/nsHTMLStyleSheet.h
layout/style/nsRuleNode.cpp
layout/style/nsStyleSet.cpp
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -234,16 +234,18 @@ nsPresContext::nsPresContext(nsIDocument
   mVisitedLinkColor = NS_RGB(0x55, 0x1A, 0x8B);
   mUnderlineLinks = PR_TRUE;
   mSendAfterPaintToContent = PR_FALSE;
 
   mFocusTextColor = mDefaultColor;
   mFocusBackgroundColor = mBackgroundColor;
   mFocusRingWidth = 1;
 
+  mBodyTextColor = mDefaultColor;
+
   if (aType == eContext_Galley) {
     mMedium = nsGkAtoms::screen;
   } else {
     mMedium = nsGkAtoms::print;
     mPaginated = PR_TRUE;
   }
 
   if (!IsDynamic()) {
@@ -708,16 +710,19 @@ nsPresContext::GetUserPreferences()
     Preferences::GetInt("browser.display.focus_ring_width", mFocusRingWidth);
 
   mFocusRingOnAnything =
     Preferences::GetBool("browser.display.focus_ring_on_anything",
                          mFocusRingOnAnything);
 
   mFocusRingStyle =
     Preferences::GetInt("browser.display.focus_ring_style", mFocusRingStyle);
+
+  mBodyTextColor = mDefaultColor;
+  
   // * use fonts?
   mUseDocumentFonts =
     Preferences::GetInt("browser.display.use_document_fonts") != 0;
 
   // * replace backslashes with Yen signs? (bug 245770)
   mEnableJapaneseTransform =
     Preferences::GetBool("layout.enable_japanese_specific_transform");
 
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -409,16 +409,22 @@ public:
   const nscolor DefaultColor() const { return mDefaultColor; }
   const nscolor DefaultBackgroundColor() const { return mBackgroundColor; }
   const nscolor DefaultLinkColor() const { return mLinkColor; }
   const nscolor DefaultActiveLinkColor() const { return mActiveLinkColor; }
   const nscolor DefaultVisitedLinkColor() const { return mVisitedLinkColor; }
   const nscolor FocusBackgroundColor() const { return mFocusBackgroundColor; }
   const nscolor FocusTextColor() const { return mFocusTextColor; }
 
+  /**
+   * Body text color, for use in quirks mode only.
+   */
+  const nscolor BodyTextColor() const { return mBodyTextColor; }
+  void SetBodyTextColor(nscolor aColor) { mBodyTextColor = aColor; }
+
   PRBool GetUseFocusColors() const { return mUseFocusColors; }
   PRUint8 FocusRingWidth() const { return mFocusRingWidth; }
   PRBool GetFocusRingOnAnything() const { return mFocusRingOnAnything; }
   PRUint8 GetFocusRingStyle() const { return mFocusRingStyle; }
 
   /**
    * The types of image load types that the pres context needs image
    * loaders to track invalidation for.
@@ -1133,16 +1139,18 @@ protected:
 
   nscolor               mLinkColor;
   nscolor               mActiveLinkColor;
   nscolor               mVisitedLinkColor;
 
   nscolor               mFocusBackgroundColor;
   nscolor               mFocusTextColor;
 
+  nscolor               mBodyTextColor;
+
   ScrollbarStyles       mViewportStyleOverflow;
   PRUint8               mFocusRingWidth;
 
   PRUint16              mImageAnimationMode;
   PRUint16              mImageAnimationModePref;
 
   nsFont                mDefaultVariableFont;
   nsFont                mDefaultFixedFont;
--- a/layout/base/nsStyleConsts.h
+++ b/layout/base/nsStyleConsts.h
@@ -228,19 +228,20 @@ static inline mozilla::css::Side operato
 #define NS_STYLE_VOLUME_SILENT            0
 #define NS_STYLE_VOLUME_X_SOFT            1
 #define NS_STYLE_VOLUME_SOFT              2
 #define NS_STYLE_VOLUME_MEDIUM            3
 #define NS_STYLE_VOLUME_LOUD              4
 #define NS_STYLE_VOLUME_X_LOUD            5
 
 // See nsStyleColor
-#define NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR      1
+#define NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR 1
+#define NS_STYLE_COLOR_INHERIT_FROM_BODY  2  /* Can't come from CSS directly */
 #ifdef GFX_HAS_INVERT
-#define NS_STYLE_COLOR_INVERT             2
+#define NS_STYLE_COLOR_INVERT             3
 #endif
 
 // See nsStyleColor
 #define NS_COLOR_CURRENTCOLOR                   -1
 #define NS_COLOR_MOZ_DEFAULT_COLOR              -2
 #define NS_COLOR_MOZ_DEFAULT_BACKGROUND_COLOR   -3
 #define NS_COLOR_MOZ_HYPERLINKTEXT              -4
 #define NS_COLOR_MOZ_VISITEDHYPERLINKTEXT       -5
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/645768-1-ref.html
@@ -0,0 +1,8 @@
+<!-- This test needs to be in quirks mode -->
+<body>
+  <div>
+    <table style="color: green">
+      <tr><td>This should be green</td></tr>
+    </table>
+  </div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/645768-1.html
@@ -0,0 +1,12 @@
+<!-- This test needs to be in quirks mode -->
+<body style="display: none; color: green">
+  <div style="color: red">
+    <table>
+      <tr><td>This should be green</td></tr>
+    </table>
+  </div>
+  <script>
+    document.body.offsetWidth;
+    document.body.style.display = "";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/655549-1-ref.html
@@ -0,0 +1,8 @@
+<!-- This test needs to be in quirks mode -->
+<body>
+  <div>
+    <table style="color: green">
+      <tr><td>This should be green</td></tr>
+    </table>
+  </div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/655549-1.html
@@ -0,0 +1,12 @@
+<!-- This test needs to be in quirks mode -->
+<body style="color: red">
+  <div>
+    <table>
+      <tr><td>This should be green</td></tr>
+    </table>
+  </div>
+  <script>
+    document.body.offsetWidth;
+    document.body.style.color = "green";
+  </script>
+</body>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1626,15 +1626,17 @@ fails-if(Android) == 635302-1.html 63530
 == 635373-1.html 635373-1-ref.html
 == 635373-2.html 635373-2-ref.html
 fails-if(http.platform=="X11"&&!layersGPUAccelerated) == 635373-3.html 635373-3-ref.html
 HTTP(..) == 635639-1.html 635639-1-ref.html
 HTTP(..) == 635639-2.html 635639-2-ref.html
 == 641770-1.html 641770-1-ref.html
 == 641856-1.html 641856-1-ref.html
 == 645491-1.html 645491-1-ref.html
+== 645768-1.html 645768-1-ref.html
 fails-if(layersGPUAccelerated&&cocoaWidget) == 650228-1.html 650228-1-ref.html # Quartz alpha blending doesn't match GL alpha blending
 == 653930-1.html 653930-1-ref.html
 HTTP(..) == 654057-1.html 654057-1-ref.html
 fails-if(layersGPUAccelerated&&cocoaWidget) == 654950-1.html 654950-1-ref.html # Quartz alpha blending doesn't match GL alpha blending
 == 652775-1.html 652775-1-ref.html
+== 655549-1.html 655549-1-ref.html
 != 656875.html about:blank
 == 658952.html 658952-ref.html
--- a/layout/style/nsHTMLStyleSheet.cpp
+++ b/layout/style/nsHTMLStyleSheet.cpp
@@ -94,22 +94,16 @@ nsHTMLStyleSheet::HTMLColorRule::MapRule
 nsHTMLStyleSheet::HTMLColorRule::List(FILE* out, PRInt32 aIndent) const
 {
 }
 #endif
 
  
 NS_IMPL_ISUPPORTS1(nsHTMLStyleSheet::GenericTableRule, nsIStyleRule)
 
-/* virtual */ void
-nsHTMLStyleSheet::GenericTableRule::MapRuleInfoInto(nsRuleData* aRuleData)
-{
-  // Nothing to do.
-}
-
 #ifdef DEBUG
 /* virtual */ void
 nsHTMLStyleSheet::GenericTableRule::List(FILE* out, PRInt32 aIndent) const
 {
 }
 #endif
 
 /* virtual */ void
@@ -119,16 +113,29 @@ nsHTMLStyleSheet::TableTHRule::MapRuleIn
     nsCSSValue* textAlign = aRuleData->ValueForTextAlign();
     if (textAlign->GetUnit() == eCSSUnit_Null) {
       textAlign->SetIntValue(NS_STYLE_TEXT_ALIGN_MOZ_CENTER_OR_INHERIT,
                              eCSSUnit_Enumerated);
     }
   }
 }
 
+/* virtual */ void
+nsHTMLStyleSheet::TableQuirkColorRule::MapRuleInfoInto(nsRuleData* aRuleData)
+{
+  if (aRuleData->mSIDs & NS_STYLE_INHERIT_BIT(Color)) {
+    nsCSSValue* color = aRuleData->ValueForColor();
+    // We do not check UseDocumentColors() here, because we want to
+    // use the body color no matter what.
+    if (color->GetUnit() == eCSSUnit_Null)
+      color->SetIntValue(NS_STYLE_COLOR_INHERIT_FROM_BODY,
+                         eCSSUnit_Enumerated);
+  }
+}
+
 // -----------------------------------------------------------
 
 struct MappedAttrTableEntry : public PLDHashEntryHdr {
   nsMappedAttributes *mAttributes;
 };
 
 static PLDHashNumber
 MappedAttrTable_HashKey(PLDHashTable *table, const void *key)
@@ -178,45 +185,28 @@ nsHTMLStyleSheet::nsHTMLStyleSheet(void)
 {
   mMappedAttrTable.ops = nsnull;
 }
 
 nsresult
 nsHTMLStyleSheet::Init()
 {
   mTableTHRule = new TableTHRule();
-  if (!mTableTHRule)
-    return NS_ERROR_OUT_OF_MEMORY;
+  mTableQuirkColorRule = new TableQuirkColorRule();
   return NS_OK;
 }
 
 nsHTMLStyleSheet::~nsHTMLStyleSheet()
 {
   if (mMappedAttrTable.ops)
     PL_DHashTableFinish(&mMappedAttrTable);
 }
 
 NS_IMPL_ISUPPORTS2(nsHTMLStyleSheet, nsIStyleSheet, nsIStyleRuleProcessor)
 
-static nsresult GetBodyColor(nsPresContext* aPresContext, nscolor* aColor)
-{
-  nsIPresShell *shell = aPresContext->PresShell();
-  nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(shell->GetDocument());
-  if (!htmlDoc)
-    return NS_ERROR_FAILURE;
-  nsIContent* bodyContent = htmlDoc->GetBodyContentExternal();
-  if (!bodyContent)
-    return NS_ERROR_FAILURE;
-  nsIFrame *bodyFrame = bodyContent->GetPrimaryFrame();
-  if (!bodyFrame)
-    return NS_ERROR_FAILURE;
-  *aColor = bodyFrame->GetStyleColor()->mColor;
-  return NS_OK;
-}
-
 /* virtual */ void
 nsHTMLStyleSheet::RulesMatching(ElementRuleProcessorData* aData)
 {
   nsRuleWalker *ruleWalker = aData->mRuleWalker;
   if (aData->mElement->IsHTML()) {
     nsIAtom* tag = aData->mElement->Tag();
 
     // if we have anchor colors, check if this is an anchor with an href
@@ -245,29 +235,17 @@ nsHTMLStyleSheet::RulesMatching(ElementR
       } // end link/visited/active rules
     } // end A tag
     // add the rule to handle text-align for a <th>
     else if (tag == nsGkAtoms::th) {
       ruleWalker->Forward(mTableTHRule);
     }
     else if (tag == nsGkAtoms::table) {
       if (aData->mTreeMatchContext.mCompatMode == eCompatibility_NavQuirks) {
-        nscolor bodyColor;
-        nsresult rv =
-          GetBodyColor(ruleWalker->CurrentNode()->GetPresContext(),
-                       &bodyColor);
-        if (NS_SUCCEEDED(rv) &&
-            (!mDocumentColorRule || bodyColor != mDocumentColorRule->mColor)) {
-          mDocumentColorRule = new HTMLColorRule();
-          if (mDocumentColorRule) {
-            mDocumentColorRule->mColor = bodyColor;
-          }
-        }
-        if (mDocumentColorRule)
-          ruleWalker->Forward(mDocumentColorRule);
+        ruleWalker->Forward(mTableQuirkColorRule);
       }
     }
   } // end html element
 
     // just get the style rules from the content
   aData->mElement->WalkContentStyleRules(ruleWalker);
 }
 
@@ -442,17 +420,16 @@ nsHTMLStyleSheet::Init(nsIURI* aURL, nsI
 void
 nsHTMLStyleSheet::Reset(nsIURI* aURL)
 {
   mURL = aURL;
 
   mLinkRule          = nsnull;
   mVisitedRule       = nsnull;
   mActiveRule        = nsnull;
-  mDocumentColorRule = nsnull;
 
   if (mMappedAttrTable.ops) {
     PL_DHashTableFinish(&mMappedAttrTable);
     mMappedAttrTable.ops = nsnull;
   }
 }
 
 nsresult
--- a/layout/style/nsHTMLStyleSheet.h
+++ b/layout/style/nsHTMLStyleSheet.h
@@ -133,38 +133,46 @@ private:
   friend class GenericTableRule;
   class GenericTableRule: public nsIStyleRule {
   public:
     GenericTableRule() {}
 
     NS_DECL_ISUPPORTS
 
     // nsIStyleRule interface
-    virtual void MapRuleInfoInto(nsRuleData* aRuleData);
+    virtual void MapRuleInfoInto(nsRuleData* aRuleData) = 0;
   #ifdef DEBUG
     virtual void List(FILE* out = stdout, PRInt32 aIndent = 0) const;
   #endif
   };
 
   // this rule handles <th> inheritance
   class TableTHRule;
   friend class TableTHRule;
   class TableTHRule: public GenericTableRule {
   public:
     TableTHRule() {}
 
     virtual void MapRuleInfoInto(nsRuleData* aRuleData);
   };
 
+  // Rule to handle quirk table colors
+  class TableQuirkColorRule : public GenericTableRule {
+  public:
+    TableQuirkColorRule() {}
+
+    virtual void MapRuleInfoInto(nsRuleData* aRuleData);
+  };
+
   nsCOMPtr<nsIURI>        mURL;
   nsIDocument*            mDocument;
   nsRefPtr<HTMLColorRule> mLinkRule;
   nsRefPtr<HTMLColorRule> mVisitedRule;
   nsRefPtr<HTMLColorRule> mActiveRule;
-  nsRefPtr<HTMLColorRule> mDocumentColorRule;
+  nsRefPtr<TableQuirkColorRule> mTableQuirkColorRule;
   nsRefPtr<TableTHRule>   mTableTHRule;
 
   PLDHashTable            mMappedAttrTable;
 };
 
 // XXX convenience method. Calls Initialize() automatically.
 nsresult
 NS_NewHTMLStyleSheet(nsHTMLStyleSheet** aInstancePtrResult, nsIURI* aURL, 
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -767,16 +767,27 @@ static PRBool SetColor(const nsCSSValue&
       result = PR_TRUE;
     }
   }
   else if (eCSSUnit_Inherit == unit) {
     aResult = aParentColor;
     result = PR_TRUE;
     aCanStoreInRuleTree = PR_FALSE;
   }
+  else if (eCSSUnit_Enumerated == unit &&
+           aValue.GetIntValue() == NS_STYLE_COLOR_INHERIT_FROM_BODY) {
+    NS_ASSERTION(aPresContext->CompatibilityMode() == eCompatibility_NavQuirks,
+                 "Should only get this value in quirks mode");
+    // We just grab the color from the prescontext, and rely on the fact that
+    // if the body color ever changes all its descendants will get new style
+    // contexts (but NOT necessarily new rulenodes).
+    aResult = aPresContext->BodyTextColor();
+    result = PR_TRUE;
+    aCanStoreInRuleTree = PR_FALSE;
+  }
   return result;
 }
 
 static void SetGradientCoord(const nsCSSValue& aValue, nsPresContext* aPresContext,
                              nsStyleContext* aContext, nsStyleCoord& aResult,
                              PRBool& aCanStoreInRuleTree)
 {
   // OK to pass bad aParentCoord since we're not passing SETCOORD_INHERIT
@@ -5336,16 +5347,19 @@ nsRuleNode::ComputeBorderData(void* aSta
                         canStoreInRuleTree)) {
         border->SetBorderColor(side, borderColor);
       }
       else if (eCSSUnit_Enumerated == value.GetUnit()) {
         switch (value.GetIntValue()) {
           case NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR:
             border->SetBorderToForeground(side);
             break;
+          default:
+            NS_NOTREACHED("Unexpected enumerated color");
+            break;
         }
       }
       else if (eCSSUnit_Initial == value.GetUnit()) {
         border->SetBorderToForeground(side);
       }
     }
   }
 
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -613,16 +613,26 @@ nsStyleSet::GetContext(nsStyleContext* a
         : nsnull;
       result = GetContext(aParentContext, ruleNode, visitedRuleNode,
                           aIsLink, aIsVisitedLink,
                           aPseudoTag, aPseudoType, PR_FALSE, nsnull);
     }
   }
 #endif
 
+  if (aElementForAnimation && aElementForAnimation->IsHTML(nsGkAtoms::body) &&
+      aPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement &&
+      PresContext()->CompatibilityMode() == eCompatibility_NavQuirks) {
+    nsIDocument* doc = aElementForAnimation->GetCurrentDoc();
+    if (doc && doc->GetBodyElement() == aElementForAnimation) {
+      // Update the prescontext's body color
+      PresContext()->SetBodyTextColor(result->GetStyleColor()->mColor);
+    }
+  }
+
   return result.forget();
 }
 
 void
 nsStyleSet::AddImportantRules(nsRuleNode* aCurrLevelNode,
                               nsRuleNode* aLastPrevLevelNode,
                               nsRuleWalker* aRuleWalker)
 {