Don't use HTML rules to parse (redundantly) media strings in CSS @import

rules.  Bug 153755, r+sr=dbaron

content/html/style/public/nsICSSLoader.h

@@ -53,6 +53,7 @@ class nsIParser;
 class nsIDocument;
 class nsIUnicharInputStream;
 class nsICSSLoaderObserver;
+class nsISupportsArray;
 
 // IID for the nsIStyleSheetLoader interface {a6cf9101-15b3-11d2-932e-00805f8add32}
 #define NS_ICSS_LOADER_IID     \
@@ -107,7 +108,7 @@ public:
   // Load a child style sheet (@import)
   NS_IMETHOD LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                             nsIURI* aURL, 
-                            const nsAString& aMedia,
+                            nsISupportsArray* aMedia,
                             nsICSSImportRule* aRule) = 0;
 
   // Load a user agent or user sheet.  The sheet is loaded

content/html/style/public/nsIMediaList.h

@@ -44,13 +44,6 @@ public:
   NS_IMETHOD MatchesMedium(nsIAtom* aMedium, PRBool* aMatch); \
   NS_IMETHOD DropReference(void);
 
-nsresult 
-NS_NewMediaList(const nsAString& aMediaText,
-                nsIMediaList** aInstancePtrResult);
-
-nsresult 
-NS_NewMediaList(nsIMediaList** aInstancePtrResult);
-
 nsresult
 NS_NewMediaList(nsISupportsArray* aArray, nsICSSStyleSheet* aSheet,
                 nsIMediaList** aInstancePtrResult);

content/html/style/src/nsCSSLoader.cpp

@@ -42,6 +42,7 @@
 #include "nsContentUtils.h"
 #include "nsCRT.h"
 #include "nsCOMArray.h"
+#include "nsISupportsArray.h"
 #include "nsCOMPtr.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsContentPolicyUtils.h"
@@ -1061,14 +1062,25 @@ CSSLoaderImpl::CreateSheet(nsIURI* aURI,
 nsresult
 CSSLoaderImpl::PrepareSheet(nsICSSStyleSheet* aSheet,
                             const nsAString& aTitle,
-                            const nsAString& aMedia)
+                            const nsAString& aMedia,
+                            nsISupportsArray* aMediaArr)
 {
   NS_PRECONDITION(aSheet, "Must have a sheet!");
+  NS_PRECONDITION(aMedia.IsEmpty() || !aMediaArr,
+                  "Can't have media array _and_ media string!");
 
   nsresult rv = NS_OK;
   aSheet->ClearMedia();
   if (!aMedia.IsEmpty()) {
     rv = EnumerateMediaString(aMedia, MediumEnumFunc, aSheet);
+  } else if (aMediaArr) {
+    PRUint32 count;
+    aMediaArr->Count(&count);
+    for (PRUint32 i = 0; i < count; ++i) {
+      nsCOMPtr<nsIAtom> medium = do_QueryElementAt(aMediaArr, i);
+      NS_ASSERTION(medium, "Null medium in media array!");
+      aSheet->AppendMedium(medium);
+    }
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1593,7 +1605,7 @@ CSSLoaderImpl::LoadInlineStyle(nsIContent* aElement,
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
-  rv = PrepareSheet(sheet, aTitle, aMedia);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
@@ -1651,7 +1663,7 @@ CSSLoaderImpl::LoadStyleLink(nsIContent* aElement,
                    getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PrepareSheet(sheet, aTitle, aMedia);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
@@ -1699,7 +1711,7 @@ CSSLoaderImpl::LoadStyleLink(nsIContent* aElement,
 NS_IMETHODIMP
 CSSLoaderImpl::LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                               nsIURI* aURL, 
-                              const nsAString& aMedia,
+                              nsISupportsArray* aMedia,
                               nsICSSImportRule* aParentRule)
 {
   LOG(("CSSLoaderImpl::LoadChildSheet"));
@@ -1760,7 +1772,8 @@ CSSLoaderImpl::LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                    state, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PrepareSheet(sheet, NS_LITERAL_STRING(""), aMedia);
+  NS_NAMED_LITERAL_STRING(empty, "");
+  rv = PrepareSheet(sheet, empty, empty, aMedia);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertChildSheet(sheet, aParentSheet, aParentRule);
@@ -1832,7 +1845,7 @@ CSSLoaderImpl::InternalLoadAgentSheet(nsIURI* aURL,
   NS_ENSURE_SUCCESS(rv, rv);
 
   NS_NAMED_LITERAL_STRING(empty, "");
-  rv = PrepareSheet(sheet, empty, empty);
+  rv = PrepareSheet(sheet, empty, empty, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   if (aSheet) {

content/html/style/src/nsCSSLoader.h

@@ -221,7 +221,7 @@ public:
 
   NS_IMETHOD LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                             nsIURI* aURL, 
-                            const nsAString& aMedia,
+                            nsISupportsArray* aMedia,
                             nsICSSImportRule* aRule);
 
   NS_IMETHOD LoadAgentSheet(nsIURI* aURL, nsICSSStyleSheet** aSheet);
@@ -260,9 +260,12 @@ private:
                        StyleSheetState& aSheetState,
                        nsICSSStyleSheet** aSheet);
 
+  // Pass in either a media string or the array of media from the
+  // CSSParser.  Don't pass both.
   nsresult PrepareSheet(nsICSSStyleSheet* aSheet,
                         const nsAString& aTitle,
-                        const nsAString& aMedia);
+                        const nsAString& aMedia,
+                        nsISupportsArray* aMediaArr);
 
   nsresult InsertSheetInDoc(nsICSSStyleSheet* aSheet,
                             nsIContent* aLinkingContent,

content/html/style/src/nsCSSParser.cpp

@@ -151,8 +151,12 @@ protected:
   PRBool ParseAtRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseCharsetRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
-  PRBool GatherMedia(nsresult& aErrorCode, nsString& aMedia, nsISupportsArray* aMediaAtoms);
-  PRBool ProcessImport(nsresult& aErrorCode, const nsString& aURLSpec, const nsString& aMedia, RuleAppendFunc aAppendFunc, void* aProcessData);
+  PRBool GatherMedia(nsresult& aErrorCode, nsISupportsArray* aMediaAtoms);
+  PRBool ProcessImport(nsresult& aErrorCode,
+                       const nsString& aURLSpec,
+                       nsISupportsArray* aMedia,
+                       RuleAppendFunc aAppendFunc,
+                       void* aProcessData);
   PRBool ParseMediaRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseNameSpaceRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ProcessNameSpace(nsresult& aErrorCode, const nsString& aPrefix, 
@@ -636,6 +640,7 @@ CSSParserImpl::ParseStyleAttribute(const nsAString& aAttributeValue,
     rv = NS_NewCSSStyleRule(&rule, nsnull, declaration);
     if (NS_FAILED(rv)) {
       declaration->RuleAbort();
+      ReleaseScanner();
       return rv;
     }
     *aResult = rule;
@@ -736,6 +741,7 @@ CSSParserImpl::ParseRule(const nsAString& aRule,
 
   rv = NS_NewISupportsArray(aResult);
   if (NS_FAILED(rv)) {
+    ReleaseScanner();
     return rv;
   }
   
@@ -1021,10 +1027,9 @@ PRBool CSSParserImpl::ParseCharsetRule(nsresult& aErrorCode, RuleAppendFunc aApp
   return PR_TRUE;
 }
 
-PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia, 
+PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode,
                                   nsISupportsArray* aMediaAtoms)
 {
-  PRBool first = PR_TRUE;
   PRBool expectIdent = PR_TRUE;
   for (;;) {
     if (!GetToken(aErrorCode, PR_TRUE)) {
@@ -1054,16 +1059,9 @@ PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia,
     }
     else if (eCSSToken_Ident == mToken.mType) {
       if (expectIdent) {
-        if (! first) {
-          aMedia.Append(PRUnichar(','));
-        }
         ToLowerCase(mToken.mIdent);  // case insensitive from CSS - must be lower cased
-        if (aMediaAtoms) {
-          nsCOMPtr<nsIAtom> medium = do_GetAtom(mToken.mIdent);
-          aMediaAtoms->AppendElement(medium);
-        }
-        aMedia.Append(mToken.mIdent);
-        first = PR_FALSE;
+        nsCOMPtr<nsIAtom> medium = do_GetAtom(mToken.mIdent);
+        aMediaAtoms->AppendElement(medium);
         expectIdent = PR_FALSE;
       }
       else {
@@ -1082,10 +1080,7 @@ PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia,
       break;
     }
   }
-  aMedia.Truncate();
-  if (aMediaAtoms) {
-    aMediaAtoms->Clear();
-  }
+  aMediaAtoms->Clear();
   return PR_FALSE;
 }
 
@@ -1097,11 +1092,16 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
     return PR_FALSE;
   }
   nsAutoString url;
-  nsAutoString media;
+  nsCOMPtr<nsISupportsArray> media;
+  aErrorCode = NS_NewISupportsArray(getter_AddRefs(media));
+  if (!media) {
+    // Out of memory
+    return PR_FALSE;
+  }
   
   if (eCSSToken_String == mToken.mType) {
     url = mToken.mIdent;
-    if (GatherMedia(aErrorCode, media, nsnull)) {
+    if (GatherMedia(aErrorCode, media)) {
       if (ExpectSymbol(aErrorCode, ';', PR_TRUE)) {
         ProcessImport(aErrorCode, url, media, aAppendFunc, aData);
         return PR_TRUE;
@@ -1115,7 +1115,7 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
         if ((eCSSToken_String == mToken.mType) || (eCSSToken_URL == mToken.mType)) {
           url = mToken.mIdent;
           if (ExpectSymbol(aErrorCode, ')', PR_TRUE)) {
-            if (GatherMedia(aErrorCode, media, nsnull)) {
+            if (GatherMedia(aErrorCode, media)) {
               if (ExpectSymbol(aErrorCode, ';', PR_TRUE)) {
                 ProcessImport(aErrorCode, url, media, aAppendFunc, aData);
                 return PR_TRUE;
@@ -1133,7 +1133,11 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
 }
 
 
-PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode, const nsString& aURLSpec, const nsString& aMedia, RuleAppendFunc aAppendFunc, void* aData)
+PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode,
+                                    const nsString& aURLSpec,
+                                    nsISupportsArray* aMedia,
+                                    RuleAppendFunc aAppendFunc,
+                                    void* aData)
 {
   nsCOMPtr<nsICSSImportRule> rule;
   aErrorCode = NS_NewCSSImportRule(getter_AddRefs(rule), aURLSpec, aMedia);
@@ -1163,12 +1167,14 @@ PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode, const nsString& aURLSp
 PRBool CSSParserImpl::ParseMediaRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc,
                                      void* aData)
 {
-  nsAutoString  mediaStr;
   nsCOMPtr<nsISupportsArray> media;
-  NS_NewISupportsArray(getter_AddRefs(media));
+  aErrorCode = NS_NewISupportsArray(getter_AddRefs(media));
   if (media) {
-    if (GatherMedia(aErrorCode, mediaStr, media)) {
-      if ((!mediaStr.IsEmpty()) &&
+    if (GatherMedia(aErrorCode, media)) {
+      // XXXbz this could use better error reporting throughout the method
+      PRUint32 count;
+      media->Count(&count);
+      if (count > 0 &&
           ExpectSymbol(aErrorCode, '{', PR_TRUE)) {
         // push media rule on stack, loop over children
         nsCOMPtr<nsICSSMediaRule>  rule;

content/html/style/src/nsCSSRules.cpp

@@ -347,7 +347,7 @@ class CSSImportRuleImpl : public nsCSSRule,
                           public nsIDOMCSSImportRule
 {
 public:
-  CSSImportRuleImpl(void);
+  CSSImportRuleImpl(nsISupportsArray* aMedia);
   CSSImportRuleImpl(const CSSImportRuleImpl& aCopy);
   virtual ~CSSImportRuleImpl(void);
 
@@ -385,11 +385,14 @@ protected:
   nsCOMPtr<nsICSSStyleSheet> mChildSheet;
 };
 
-CSSImportRuleImpl::CSSImportRuleImpl(void)
+CSSImportRuleImpl::CSSImportRuleImpl(nsISupportsArray* aMedia)
   : nsCSSRule(),
     mURLSpec()
 {
-  NS_NewMediaList(getter_AddRefs(mMedia));
+  // XXXbz This is really silly.... the mMedia we create here will be
+  // clobbered if we manage to load a sheet.  Which should really
+  // never fail nowadays, in sane cases.
+  NS_NewMediaList(aMedia, nsnull, getter_AddRefs(mMedia));
 }
 
 CSSImportRuleImpl::CSSImportRuleImpl(const CSSImportRuleImpl& aCopy)
@@ -397,16 +400,12 @@ CSSImportRuleImpl::CSSImportRuleImpl(const CSSImportRuleImpl& aCopy)
     mURLSpec(aCopy.mURLSpec)
 {
   
+  nsCOMPtr<nsICSSStyleSheet> sheet;
   if (aCopy.mChildSheet) {
     aCopy.mChildSheet->Clone(nsnull, this, nsnull, nsnull,
-                             getter_AddRefs(mChildSheet));
-  }
-
-  NS_NewMediaList(getter_AddRefs(mMedia));
-  
-  if (aCopy.mMedia && mMedia) {
-    mMedia->AppendElement(aCopy.mMedia);
+                             getter_AddRefs(sheet));
   }
+  SetSheet(sheet);
 }
 
 CSSImportRuleImpl::~CSSImportRuleImpl(void)
@@ -529,18 +528,17 @@ CSSImportRuleImpl::SetSheet(nsICSSStyleSheet* aSheet)
 nsresult
 NS_NewCSSImportRule(nsICSSImportRule** aInstancePtrResult, 
                     const nsString& aURLSpec,
-                    const nsString& aMedia)
+                    nsISupportsArray* aMedia)
 {
   NS_ENSURE_ARG_POINTER(aInstancePtrResult);
 
-  CSSImportRuleImpl* it = new CSSImportRuleImpl();
+  CSSImportRuleImpl* it = new CSSImportRuleImpl(aMedia);
 
   if (! it) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   it->SetURLSpec(aURLSpec);
-  it->SetMedia(aMedia);
   return CallQueryInterface(it, aInstancePtrResult);
 }
 

content/html/style/src/nsCSSStyleSheet.cpp

@@ -1074,33 +1074,9 @@ DOMMediaListImpl::~DOMMediaListImpl()
 }
 
 nsresult
-NS_NewMediaList(nsIMediaList** aInstancePtrResult) {
-  return NS_NewMediaList(NS_LITERAL_STRING(""), aInstancePtrResult);
-}
-
-nsresult
-NS_NewMediaList(const nsAString& aMediaText, nsIMediaList** aInstancePtrResult) {
-  nsresult rv;
-  NS_ASSERTION(aInstancePtrResult, "Null out param.");
-
-  nsCOMPtr<nsISupportsArray> array;
-  rv = NS_NewISupportsArray(getter_AddRefs(array));
-  if (NS_FAILED(rv)) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  DOMMediaListImpl* medialist = new DOMMediaListImpl(array, nsnull);
-  *aInstancePtrResult = medialist;
-  NS_ENSURE_TRUE(medialist, NS_ERROR_OUT_OF_MEMORY);
-  NS_ADDREF(*aInstancePtrResult);
-  rv = medialist->SetMediaText(aMediaText);
-  if (NS_FAILED(rv)) {
-    NS_RELEASE(*aInstancePtrResult);
-    *aInstancePtrResult = nsnull;
-  }
-  return rv;
-}
-
-nsresult NS_NewMediaList(nsISupportsArray* aArray, nsICSSStyleSheet* aSheet, nsIMediaList** aInstancePtrResult)
+NS_NewMediaList(nsISupportsArray* aArray,
+                nsICSSStyleSheet* aSheet,
+                nsIMediaList** aInstancePtrResult)
 {
   NS_ASSERTION(aInstancePtrResult, "Null out param.");
   DOMMediaListImpl* medialist = new DOMMediaListImpl(aArray, NS_STATIC_CAST(CSSStyleSheetImpl*, aSheet));
@@ -1137,6 +1113,8 @@ DOMMediaListImpl::GetText(nsAString& aMediaText)
   return NS_OK;
 }
 
+// XXXbz this is so ill-defined in the spec, it's not clear quite what
+// it should be doing....
 NS_IMETHODIMP
 DOMMediaListImpl::SetText(const nsAString& aMediaText)
 {

content/html/style/src/nsICSSImportRule.h

@@ -43,6 +43,7 @@
 
 class nsIAtom;
 class nsIURI;
+class nsISupportsArray;
 
 // IID for the nsICSSImportRule interface {33824a60-1a09-11d3-805a-006008159b5a}
 #define NS_ICSS_IMPORT_RULE_IID     \
@@ -63,6 +64,6 @@ public:
 
 nsresult
 NS_NewCSSImportRule(nsICSSImportRule** aInstancePtrResult, 
-                    const nsString& aURLSpec, const nsString& aMedia);
+                    const nsString& aURLSpec, nsISupportsArray* aMedia);
 
 #endif /* nsICSSImportRule_h___ */

layout/style/nsCSSLoader.cpp

@@ -42,6 +42,7 @@
 #include "nsContentUtils.h"
 #include "nsCRT.h"
 #include "nsCOMArray.h"
+#include "nsISupportsArray.h"
 #include "nsCOMPtr.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsContentPolicyUtils.h"
@@ -1061,14 +1062,25 @@ CSSLoaderImpl::CreateSheet(nsIURI* aURI,
 nsresult
 CSSLoaderImpl::PrepareSheet(nsICSSStyleSheet* aSheet,
                             const nsAString& aTitle,
-                            const nsAString& aMedia)
+                            const nsAString& aMedia,
+                            nsISupportsArray* aMediaArr)
 {
   NS_PRECONDITION(aSheet, "Must have a sheet!");
+  NS_PRECONDITION(aMedia.IsEmpty() || !aMediaArr,
+                  "Can't have media array _and_ media string!");
 
   nsresult rv = NS_OK;
   aSheet->ClearMedia();
   if (!aMedia.IsEmpty()) {
     rv = EnumerateMediaString(aMedia, MediumEnumFunc, aSheet);
+  } else if (aMediaArr) {
+    PRUint32 count;
+    aMediaArr->Count(&count);
+    for (PRUint32 i = 0; i < count; ++i) {
+      nsCOMPtr<nsIAtom> medium = do_QueryElementAt(aMediaArr, i);
+      NS_ASSERTION(medium, "Null medium in media array!");
+      aSheet->AppendMedium(medium);
+    }
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1593,7 +1605,7 @@ CSSLoaderImpl::LoadInlineStyle(nsIContent* aElement,
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
-  rv = PrepareSheet(sheet, aTitle, aMedia);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
@@ -1651,7 +1663,7 @@ CSSLoaderImpl::LoadStyleLink(nsIContent* aElement,
                    getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PrepareSheet(sheet, aTitle, aMedia);
+  rv = PrepareSheet(sheet, aTitle, aMedia, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertSheetInDoc(sheet, aElement, mDocument);
@@ -1699,7 +1711,7 @@ CSSLoaderImpl::LoadStyleLink(nsIContent* aElement,
 NS_IMETHODIMP
 CSSLoaderImpl::LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                               nsIURI* aURL, 
-                              const nsAString& aMedia,
+                              nsISupportsArray* aMedia,
                               nsICSSImportRule* aParentRule)
 {
   LOG(("CSSLoaderImpl::LoadChildSheet"));
@@ -1760,7 +1772,8 @@ CSSLoaderImpl::LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                    state, getter_AddRefs(sheet));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = PrepareSheet(sheet, NS_LITERAL_STRING(""), aMedia);
+  NS_NAMED_LITERAL_STRING(empty, "");
+  rv = PrepareSheet(sheet, empty, empty, aMedia);
   NS_ENSURE_SUCCESS(rv, rv);
   
   rv = InsertChildSheet(sheet, aParentSheet, aParentRule);
@@ -1832,7 +1845,7 @@ CSSLoaderImpl::InternalLoadAgentSheet(nsIURI* aURL,
   NS_ENSURE_SUCCESS(rv, rv);
 
   NS_NAMED_LITERAL_STRING(empty, "");
-  rv = PrepareSheet(sheet, empty, empty);
+  rv = PrepareSheet(sheet, empty, empty, nsnull);
   NS_ENSURE_SUCCESS(rv, rv);
   
   if (aSheet) {

layout/style/nsCSSLoader.h

@@ -221,7 +221,7 @@ public:
 
   NS_IMETHOD LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                             nsIURI* aURL, 
-                            const nsAString& aMedia,
+                            nsISupportsArray* aMedia,
                             nsICSSImportRule* aRule);
 
   NS_IMETHOD LoadAgentSheet(nsIURI* aURL, nsICSSStyleSheet** aSheet);
@@ -260,9 +260,12 @@ private:
                        StyleSheetState& aSheetState,
                        nsICSSStyleSheet** aSheet);
 
+  // Pass in either a media string or the array of media from the
+  // CSSParser.  Don't pass both.
   nsresult PrepareSheet(nsICSSStyleSheet* aSheet,
                         const nsAString& aTitle,
-                        const nsAString& aMedia);
+                        const nsAString& aMedia,
+                        nsISupportsArray* aMediaArr);
 
   nsresult InsertSheetInDoc(nsICSSStyleSheet* aSheet,
                             nsIContent* aLinkingContent,

layout/style/nsCSSParser.cpp

@@ -151,8 +151,12 @@ protected:
   PRBool ParseAtRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseCharsetRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
-  PRBool GatherMedia(nsresult& aErrorCode, nsString& aMedia, nsISupportsArray* aMediaAtoms);
-  PRBool ProcessImport(nsresult& aErrorCode, const nsString& aURLSpec, const nsString& aMedia, RuleAppendFunc aAppendFunc, void* aProcessData);
+  PRBool GatherMedia(nsresult& aErrorCode, nsISupportsArray* aMediaAtoms);
+  PRBool ProcessImport(nsresult& aErrorCode,
+                       const nsString& aURLSpec,
+                       nsISupportsArray* aMedia,
+                       RuleAppendFunc aAppendFunc,
+                       void* aProcessData);
   PRBool ParseMediaRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ParseNameSpaceRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc, void* aProcessData);
   PRBool ProcessNameSpace(nsresult& aErrorCode, const nsString& aPrefix, 
@@ -636,6 +640,7 @@ CSSParserImpl::ParseStyleAttribute(const nsAString& aAttributeValue,
     rv = NS_NewCSSStyleRule(&rule, nsnull, declaration);
     if (NS_FAILED(rv)) {
       declaration->RuleAbort();
+      ReleaseScanner();
       return rv;
     }
     *aResult = rule;
@@ -736,6 +741,7 @@ CSSParserImpl::ParseRule(const nsAString& aRule,
 
   rv = NS_NewISupportsArray(aResult);
   if (NS_FAILED(rv)) {
+    ReleaseScanner();
     return rv;
   }
   
@@ -1021,10 +1027,9 @@ PRBool CSSParserImpl::ParseCharsetRule(nsresult& aErrorCode, RuleAppendFunc aApp
   return PR_TRUE;
 }
 
-PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia, 
+PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode,
                                   nsISupportsArray* aMediaAtoms)
 {
-  PRBool first = PR_TRUE;
   PRBool expectIdent = PR_TRUE;
   for (;;) {
     if (!GetToken(aErrorCode, PR_TRUE)) {
@@ -1054,16 +1059,9 @@ PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia,
     }
     else if (eCSSToken_Ident == mToken.mType) {
       if (expectIdent) {
-        if (! first) {
-          aMedia.Append(PRUnichar(','));
-        }
         ToLowerCase(mToken.mIdent);  // case insensitive from CSS - must be lower cased
-        if (aMediaAtoms) {
-          nsCOMPtr<nsIAtom> medium = do_GetAtom(mToken.mIdent);
-          aMediaAtoms->AppendElement(medium);
-        }
-        aMedia.Append(mToken.mIdent);
-        first = PR_FALSE;
+        nsCOMPtr<nsIAtom> medium = do_GetAtom(mToken.mIdent);
+        aMediaAtoms->AppendElement(medium);
         expectIdent = PR_FALSE;
       }
       else {
@@ -1082,10 +1080,7 @@ PRBool CSSParserImpl::GatherMedia(nsresult& aErrorCode, nsString& aMedia,
       break;
     }
   }
-  aMedia.Truncate();
-  if (aMediaAtoms) {
-    aMediaAtoms->Clear();
-  }
+  aMediaAtoms->Clear();
   return PR_FALSE;
 }
 
@@ -1097,11 +1092,16 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
     return PR_FALSE;
   }
   nsAutoString url;
-  nsAutoString media;
+  nsCOMPtr<nsISupportsArray> media;
+  aErrorCode = NS_NewISupportsArray(getter_AddRefs(media));
+  if (!media) {
+    // Out of memory
+    return PR_FALSE;
+  }
   
   if (eCSSToken_String == mToken.mType) {
     url = mToken.mIdent;
-    if (GatherMedia(aErrorCode, media, nsnull)) {
+    if (GatherMedia(aErrorCode, media)) {
       if (ExpectSymbol(aErrorCode, ';', PR_TRUE)) {
         ProcessImport(aErrorCode, url, media, aAppendFunc, aData);
         return PR_TRUE;
@@ -1115,7 +1115,7 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
         if ((eCSSToken_String == mToken.mType) || (eCSSToken_URL == mToken.mType)) {
           url = mToken.mIdent;
           if (ExpectSymbol(aErrorCode, ')', PR_TRUE)) {
-            if (GatherMedia(aErrorCode, media, nsnull)) {
+            if (GatherMedia(aErrorCode, media)) {
               if (ExpectSymbol(aErrorCode, ';', PR_TRUE)) {
                 ProcessImport(aErrorCode, url, media, aAppendFunc, aData);
                 return PR_TRUE;
@@ -1133,7 +1133,11 @@ PRBool CSSParserImpl::ParseImportRule(nsresult& aErrorCode, RuleAppendFunc aAppe
 }
 
 
-PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode, const nsString& aURLSpec, const nsString& aMedia, RuleAppendFunc aAppendFunc, void* aData)
+PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode,
+                                    const nsString& aURLSpec,
+                                    nsISupportsArray* aMedia,
+                                    RuleAppendFunc aAppendFunc,
+                                    void* aData)
 {
   nsCOMPtr<nsICSSImportRule> rule;
   aErrorCode = NS_NewCSSImportRule(getter_AddRefs(rule), aURLSpec, aMedia);
@@ -1163,12 +1167,14 @@ PRBool CSSParserImpl::ProcessImport(nsresult& aErrorCode, const nsString& aURLSp
 PRBool CSSParserImpl::ParseMediaRule(nsresult& aErrorCode, RuleAppendFunc aAppendFunc,
                                      void* aData)
 {
-  nsAutoString  mediaStr;
   nsCOMPtr<nsISupportsArray> media;
-  NS_NewISupportsArray(getter_AddRefs(media));
+  aErrorCode = NS_NewISupportsArray(getter_AddRefs(media));
   if (media) {
-    if (GatherMedia(aErrorCode, mediaStr, media)) {
-      if ((!mediaStr.IsEmpty()) &&
+    if (GatherMedia(aErrorCode, media)) {
+      // XXXbz this could use better error reporting throughout the method
+      PRUint32 count;
+      media->Count(&count);
+      if (count > 0 &&
           ExpectSymbol(aErrorCode, '{', PR_TRUE)) {
         // push media rule on stack, loop over children
         nsCOMPtr<nsICSSMediaRule>  rule;

layout/style/nsCSSRules.cpp

@@ -347,7 +347,7 @@ class CSSImportRuleImpl : public nsCSSRule,
                           public nsIDOMCSSImportRule
 {
 public:
-  CSSImportRuleImpl(void);
+  CSSImportRuleImpl(nsISupportsArray* aMedia);
   CSSImportRuleImpl(const CSSImportRuleImpl& aCopy);
   virtual ~CSSImportRuleImpl(void);
 
@@ -385,11 +385,14 @@ protected:
   nsCOMPtr<nsICSSStyleSheet> mChildSheet;
 };
 
-CSSImportRuleImpl::CSSImportRuleImpl(void)
+CSSImportRuleImpl::CSSImportRuleImpl(nsISupportsArray* aMedia)
   : nsCSSRule(),
     mURLSpec()
 {
-  NS_NewMediaList(getter_AddRefs(mMedia));
+  // XXXbz This is really silly.... the mMedia we create here will be
+  // clobbered if we manage to load a sheet.  Which should really
+  // never fail nowadays, in sane cases.
+  NS_NewMediaList(aMedia, nsnull, getter_AddRefs(mMedia));
 }
 
 CSSImportRuleImpl::CSSImportRuleImpl(const CSSImportRuleImpl& aCopy)
@@ -397,16 +400,12 @@ CSSImportRuleImpl::CSSImportRuleImpl(const CSSImportRuleImpl& aCopy)
     mURLSpec(aCopy.mURLSpec)
 {
   
+  nsCOMPtr<nsICSSStyleSheet> sheet;
   if (aCopy.mChildSheet) {
     aCopy.mChildSheet->Clone(nsnull, this, nsnull, nsnull,
-                             getter_AddRefs(mChildSheet));
-  }
-
-  NS_NewMediaList(getter_AddRefs(mMedia));
-  
-  if (aCopy.mMedia && mMedia) {
-    mMedia->AppendElement(aCopy.mMedia);
+                             getter_AddRefs(sheet));
   }
+  SetSheet(sheet);
 }
 
 CSSImportRuleImpl::~CSSImportRuleImpl(void)
@@ -529,18 +528,17 @@ CSSImportRuleImpl::SetSheet(nsICSSStyleSheet* aSheet)
 nsresult
 NS_NewCSSImportRule(nsICSSImportRule** aInstancePtrResult, 
                     const nsString& aURLSpec,
-                    const nsString& aMedia)
+                    nsISupportsArray* aMedia)
 {
   NS_ENSURE_ARG_POINTER(aInstancePtrResult);
 
-  CSSImportRuleImpl* it = new CSSImportRuleImpl();
+  CSSImportRuleImpl* it = new CSSImportRuleImpl(aMedia);
 
   if (! it) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   it->SetURLSpec(aURLSpec);
-  it->SetMedia(aMedia);
   return CallQueryInterface(it, aInstancePtrResult);
 }
 

layout/style/nsCSSStyleSheet.cpp

@@ -1074,33 +1074,9 @@ DOMMediaListImpl::~DOMMediaListImpl()
 }
 
 nsresult
-NS_NewMediaList(nsIMediaList** aInstancePtrResult) {
-  return NS_NewMediaList(NS_LITERAL_STRING(""), aInstancePtrResult);
-}
-
-nsresult
-NS_NewMediaList(const nsAString& aMediaText, nsIMediaList** aInstancePtrResult) {
-  nsresult rv;
-  NS_ASSERTION(aInstancePtrResult, "Null out param.");
-
-  nsCOMPtr<nsISupportsArray> array;
-  rv = NS_NewISupportsArray(getter_AddRefs(array));
-  if (NS_FAILED(rv)) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  DOMMediaListImpl* medialist = new DOMMediaListImpl(array, nsnull);
-  *aInstancePtrResult = medialist;
-  NS_ENSURE_TRUE(medialist, NS_ERROR_OUT_OF_MEMORY);
-  NS_ADDREF(*aInstancePtrResult);
-  rv = medialist->SetMediaText(aMediaText);
-  if (NS_FAILED(rv)) {
-    NS_RELEASE(*aInstancePtrResult);
-    *aInstancePtrResult = nsnull;
-  }
-  return rv;
-}
-
-nsresult NS_NewMediaList(nsISupportsArray* aArray, nsICSSStyleSheet* aSheet, nsIMediaList** aInstancePtrResult)
+NS_NewMediaList(nsISupportsArray* aArray,
+                nsICSSStyleSheet* aSheet,
+                nsIMediaList** aInstancePtrResult)
 {
   NS_ASSERTION(aInstancePtrResult, "Null out param.");
   DOMMediaListImpl* medialist = new DOMMediaListImpl(aArray, NS_STATIC_CAST(CSSStyleSheetImpl*, aSheet));
@@ -1137,6 +1113,8 @@ DOMMediaListImpl::GetText(nsAString& aMediaText)
   return NS_OK;
 }
 
+// XXXbz this is so ill-defined in the spec, it's not clear quite what
+// it should be doing....
 NS_IMETHODIMP
 DOMMediaListImpl::SetText(const nsAString& aMediaText)
 {

layout/style/nsICSSImportRule.h

@@ -43,6 +43,7 @@
 
 class nsIAtom;
 class nsIURI;
+class nsISupportsArray;
 
 // IID for the nsICSSImportRule interface {33824a60-1a09-11d3-805a-006008159b5a}
 #define NS_ICSS_IMPORT_RULE_IID     \
@@ -63,6 +64,6 @@ public:
 
 nsresult
 NS_NewCSSImportRule(nsICSSImportRule** aInstancePtrResult, 
-                    const nsString& aURLSpec, const nsString& aMedia);
+                    const nsString& aURLSpec, nsISupportsArray* aMedia);
 
 #endif /* nsICSSImportRule_h___ */

layout/style/nsICSSLoader.h

@@ -53,6 +53,7 @@ class nsIParser;
 class nsIDocument;
 class nsIUnicharInputStream;
 class nsICSSLoaderObserver;
+class nsISupportsArray;
 
 // IID for the nsIStyleSheetLoader interface {a6cf9101-15b3-11d2-932e-00805f8add32}
 #define NS_ICSS_LOADER_IID     \
@@ -107,7 +108,7 @@ public:
   // Load a child style sheet (@import)
   NS_IMETHOD LoadChildSheet(nsICSSStyleSheet* aParentSheet,
                             nsIURI* aURL, 
-                            const nsAString& aMedia,
+                            nsISupportsArray* aMedia,
                             nsICSSImportRule* aRule) = 0;
 
   // Load a user agent or user sheet.  The sheet is loaded

layout/style/nsIMediaList.h

@@ -44,13 +44,6 @@ public:
   NS_IMETHOD MatchesMedium(nsIAtom* aMedium, PRBool* aMatch); \
   NS_IMETHOD DropReference(void);
 
-nsresult 
-NS_NewMediaList(const nsAString& aMediaText,
-                nsIMediaList** aInstancePtrResult);
-
-nsresult 
-NS_NewMediaList(nsIMediaList** aInstancePtrResult);
-
 nsresult
 NS_NewMediaList(nsISupportsArray* aArray, nsICSSStyleSheet* aSheet,
                 nsIMediaList** aInstancePtrResult);