Bug 1339252: Allow comments in SVG presentation attributes; r?bz draft
authorManish Goregaokar <manishearth@gmail.com>
Mon, 13 Feb 2017 16:02:46 -0800
changeset 483582 4749d938b3cb20f80634422c447e9b737c4901df
parent 482503 00d16f03506b7f9f754b01a0a458c05445ac6dba
child 545671 0138b9fd9c996631d6403c693d1a0ff4ae03ad29
push id45347
push userbmo:manishearth@gmail.com
push dateTue, 14 Feb 2017 15:17:34 +0000
reviewersbz
bugs1339252
milestone54.0a1
Bug 1339252: Allow comments in SVG presentation attributes; r?bz MozReview-Commit-ID: AfxWHvC5Byw
layout/reftests/svg/comments-in-pres-attrs.svg
layout/reftests/svg/cssComment-in-attribute-01-ref.svg
layout/reftests/svg/cssComment-in-attribute-01.svg
layout/reftests/svg/reftest.list
layout/style/nsCSSParser.cpp
layout/style/nsCSSScanner.cpp
layout/style/nsCSSScanner.h
testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/comments-in-pres-attrs.svg
@@ -0,0 +1,8 @@
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1">
+  <title>Test for whether comments are allowed in SVG presentation attributes</title>
+  <rect width="100%" height="100%" fill="/* comment */lime/* also comment */"/>
+</svg>
deleted file mode 100644
--- a/layout/reftests/svg/cssComment-in-attribute-01-ref.svg
+++ /dev/null
@@ -1,4 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg">
-  <title>Reference that css comment in attribute is not allowed</title>
-  <rect width="100%" height="100%" fill="black"/>
-</svg>
deleted file mode 100644
--- a/layout/reftests/svg/cssComment-in-attribute-01.svg
+++ /dev/null
@@ -1,4 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg">
-  <title>Testcase that css comment in attribute is not allowed</title>
-  <rect width="100%" height="100%" fill="/* blah */ red"/>
-</svg>
--- a/layout/reftests/svg/reftest.list
+++ b/layout/reftests/svg/reftest.list
@@ -22,32 +22,32 @@ include load-only/reftest.list
 # Mozilla only tests (i.e. those containing XUL/XBL/etc.)
 include moz-only/reftest.list
 
 # svg-integration tests (using svg effects in e.g. HTML)
 include svg-integration/reftest.list
 
 == baseline-middle-01.svg pass.svg
 == border-radius-01.html pass.svg
-== cssComment-in-attribute-01.svg cssComment-in-attribute-01-ref.svg
 == clip-01.svg pass.svg
 == clip-02a.svg clip-02-ref.svg
 == clip-02b.svg clip-02-ref.svg
 == clipPath-advanced-01.svg pass.svg
 fuzzy-if(/^Windows\x20NT\x2010\.0/.test(http.oscpu)||/^Windows\x20NT\x206\.[12]/.test(http.oscpu),1,5) fuzzy-if(OSX,1,6) fuzzy-if(skiaContent,1,630) == clipPath-and-shape-rendering-01.svg clipPath-and-shape-rendering-01-ref.svg # bug 614840
 == clipPath-and-transform-01.svg pass.svg
 == clipPath-basic-01.svg pass.svg
 == clipPath-basic-02.svg pass.svg
 == clipPath-basic-03.svg pass.svg
 == clipPath-basic-04.svg pass.svg
 == clipPath-basic-05.svg pass.svg
 == clipPath-basic-06.svg pass.svg
 == clipPath-basic-07.svg pass.svg
 == clipPath-winding-01.svg pass.svg
 == clip-surface-clone-01.svg clip-surface-clone-01-ref.svg
+== comments-in-pres-attrs.svg pass.svg
 == conditions-01.svg pass.svg
 == conditions-02.svg pass.svg
 == conditions-03.svg pass.svg
 == conditions-04.svg pass.svg
 == conditions-05.svg about:blank
 == conditions-07.svg pass.svg
 fuzzy-if(skiaContent,1,320) == conditions-08.svg conditions-08-ref.svg
 == conditions-09.svg conditions-09-ref.svg
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -591,21 +591,16 @@ protected:
       MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
       CSSParserImpl* mParser;
       nsCSSScanner *mOriginalScanner;
       nsCSSScanner mStringScanner;
       nsAutoCSSParserInputStateRestorer mParserStateRestorer;
       nsAutoSuppressErrors mErrorSuppresser;
   };
 
-
-  bool IsSVGMode() const {
-    return mScanner->IsSVGMode();
-  }
-
   /**
    * Saves the current input state, which includes any currently pushed
    * back token, and the current position of the scanner.
    */
   void SaveInputState(CSSParserInputState& aState);
 
   /**
    * Restores the saved input state by pushing back any saved pushback
@@ -1459,16 +1454,20 @@ protected:
   // useful in user sheets.  The only values stored in this field are
   // 0, 1, and 2; three bits are allocated to avoid issues should the
   // enum type be signed.
   css::SheetParsingMode mParsingMode : 3;
 
   // True if we are in parsing rules for the chrome.
   bool mIsChrome : 1;
 
+  // True if we're parsing SVG presentation attributes
+  // These attributes allow non-calc lengths to be unitless (mapping to px)
+  bool mIsSVGMode : 1;
+
   // True if viewport units should be allowed.
   bool mViewportUnitsEnabled : 1;
 
   // This flag is set when parsing a non-box shorthand; it's used to not apply
   // some quirks during shorthand parsing
   bool          mParsingCompoundProperty : 1;
 
   // True if we are in the middle of parsing an @supports condition.
@@ -1581,16 +1580,17 @@ CSSParserImpl::CSSParserImpl()
     mSection(eCSSSection_Charset),
     mNameSpaceMap(nullptr),
     mHavePushBack(false),
     mNavQuirkMode(false),
     mHashlessColorQuirk(false),
     mUnitlessLengthQuirk(false),
     mParsingMode(css::eAuthorSheetFeatures),
     mIsChrome(false),
+    mIsSVGMode(false),
     mViewportUnitsEnabled(true),
     mParsingCompoundProperty(false),
     mInSupportsCondition(false),
     mInFailingSupportsRule(false),
     mSuppressErrors(false),
     mSheetPrincipalRequired(true),
     mWebkitBoxUnprefixState(eNotParsingDecls),
     mNextFree(nullptr)
@@ -1672,16 +1672,17 @@ CSSParserImpl::InitScanner(nsCSSScanner&
   mSheetPrincipal = aSheetPrincipal;
   mHavePushBack = false;
 }
 
 void
 CSSParserImpl::ReleaseScanner()
 {
   mScanner = nullptr;
+  mIsSVGMode = false;
   mReporter = nullptr;
   mBaseURI = nullptr;
   mSheetURI = nullptr;
   mSheetPrincipal = nullptr;
 }
 
 nsresult
 CSSParserImpl::ParseSheet(const nsAString& aInput,
@@ -1986,31 +1987,31 @@ CSSParserImpl::ParseProperty(const nsCSS
   mData.AssertInitialState();
   mTempData.AssertInitialState();
   aDeclaration->AssertMutable();
 
   nsCSSScanner scanner(aPropValue, 0);
   css::ErrorReporter reporter(scanner, mSheet, mChildLoader, aSheetURI);
   InitScanner(scanner, reporter, aSheetURI, aBaseURI, aSheetPrincipal);
   mSection = eCSSSection_General;
-  scanner.SetSVGMode(aIsSVGMode);
 
   *aChanged = false;
 
   // Check for unknown or preffed off properties
   if (eCSSProperty_UNKNOWN == aPropID ||
       !nsCSSProps::IsEnabled(aPropID, EnabledState())) {
     NS_ConvertASCIItoUTF16 propName(nsCSSProps::GetStringValue(aPropID));
     REPORT_UNEXPECTED_P(PEUnknownProperty, propName);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
     ReleaseScanner();
     return;
   }
 
+  mIsSVGMode = aIsSVGMode;
   bool parsedOK = ParseProperty(aPropID);
   // We should now be at EOF
   if (parsedOK && GetToken(true)) {
     REPORT_UNEXPECTED_TOKEN(PEExpectEndValue);
     parsedOK = false;
   }
 
   if (!parsedOK) {
@@ -2036,16 +2037,17 @@ CSSParserImpl::ParseProperty(const nsCSS
                                           true, false, aDeclaration,
                                           GetDocument());
       aDeclaration->CompressFrom(&mData);
     }
     CLEAR_ERROR();
   }
 
   mTempData.AssertInitialState();
+  mIsSVGMode = false;
 
   ReleaseScanner();
 }
 
 void
 CSSParserImpl::ParseVariable(const nsAString& aVariableName,
                              const nsAString& aPropValue,
                              nsIURI* aSheetURI,
@@ -7786,17 +7788,17 @@ CSSParserImpl::ParseVariant(nsCSSValue& 
   if (mUnitlessLengthQuirk) { // NONSTANDARD: Nav interprets unitless numbers as px
     if (((aVariantMask & VARIANT_LENGTH) != 0) &&
         (eCSSToken_Number == tk->mType)) {
       aValue.SetFloatValue(tk->mNumber, eCSSUnit_Pixel);
       return CSSParseResult::Ok;
     }
   }
 
-  if (IsSVGMode() && !IsParsingCompoundProperty()) {
+  if (mIsSVGMode && !IsParsingCompoundProperty()) {
     // STANDARD: SVG Spec states that lengths and coordinates can be unitless
     // in which case they default to user-units (1 px = 1 user unit)
     if (((aVariantMask & VARIANT_LENGTH) != 0) &&
         (eCSSToken_Number == tk->mType)) {
       aValue.SetFloatValue(tk->mNumber, eCSSUnit_Pixel);
       return CSSParseResult::Ok;
     }
   }
@@ -17768,17 +17770,16 @@ CSSParserImpl::IsValueValidForProperty(c
   // SetValueToURL, we set mSheetPrincipalRequired to false to declare
   // that it's safe to skip the assertion.
   AutoRestore<bool> autoRestore(mSheetPrincipalRequired);
   mSheetPrincipalRequired = false;
 
   nsAutoSuppressErrors suppressErrors(this);
 
   mSection = eCSSSection_General;
-  scanner.SetSVGMode(false);
 
   // Check for unknown properties
   if (eCSSProperty_UNKNOWN == aPropID) {
     ReleaseScanner();
     return false;
   }
 
   // Check that the property and value parse successfully
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -347,17 +347,16 @@ nsCSSScanner::nsCSSScanner(const nsAStri
   , mLineNumber(aLineNumber)
   , mLineOffset(0)
   , mTokenLineNumber(aLineNumber)
   , mTokenLineOffset(0)
   , mTokenOffset(0)
   , mRecordStartOffset(0)
   , mEOFCharacters(eEOFCharacters_None)
   , mReporter(nullptr)
-  , mSVGMode(false)
   , mRecording(false)
   , mSeenBadToken(false)
   , mSeenVariableReference(false)
 {
   MOZ_COUNT_CTOR(nsCSSScanner);
 }
 
 nsCSSScanner::~nsCSSScanner()
@@ -1228,17 +1227,17 @@ nsCSSScanner::Next(nsCSSToken& aToken, n
     if (IsWhitespace(ch)) {
       SkipWhitespace();
       if (aSkip != eCSSScannerExclude_WhitespaceAndComments) {
         aToken.mType = eCSSToken_Whitespace;
         return true;
       }
       continue; // start again at the beginning
     }
-    if (ch == '/' && !IsSVGMode() && Peek(1) == '*') {
+    if (ch == '/' && Peek(1) == '*') {
       SkipComment();
       if (aSkip == eCSSScannerExclude_None) {
         aToken.mType = eCSSToken_Comment;
         return true;
       }
       continue; // start again at the beginning
     }
     break;
--- a/layout/style/nsCSSScanner.h
+++ b/layout/style/nsCSSScanner.h
@@ -203,23 +203,16 @@ class nsCSSScanner {
   // ownership of |aBuffer|, so the caller must be sure to keep it
   // alive for the lifetime of the scanner.
   nsCSSScanner(const nsAString& aBuffer, uint32_t aLineNumber);
   ~nsCSSScanner();
 
   void SetErrorReporter(mozilla::css::ErrorReporter* aReporter) {
     mReporter = aReporter;
   }
-  // Set whether or not we are processing SVG
-  void SetSVGMode(bool aSVGMode) {
-    mSVGMode = aSVGMode;
-  }
-  bool IsSVGMode() const {
-    return mSVGMode;
-  }
 
   // Reset or check whether a BAD_URL or BAD_STRING token has been seen.
   void ClearSeenBadToken() { mSeenBadToken = false; }
   bool SeenBadToken() const { return mSeenBadToken; }
 
   // Reset or check whether a "var(" FUNCTION token has been seen.
   void ClearSeenVariableReference() { mSeenVariableReference = false; }
   bool SeenVariableReference() const { return mSeenVariableReference; }
@@ -360,18 +353,16 @@ protected:
   uint32_t mTokenLineOffset;
   uint32_t mTokenOffset;
 
   uint32_t mRecordStartOffset;
   EOFCharacters mEOFCharacters;
 
   mozilla::css::ErrorReporter *mReporter;
 
-  // True if we are in SVG mode; false in "normal" CSS
-  bool mSVGMode;
   bool mRecording;
   bool mSeenBadToken;
   bool mSeenVariableReference;
 };
 
 // Token for the grid-template-areas micro-syntax
 // http://dev.w3.org/csswg/css-grid/#propdef-grid-template-areas
 struct MOZ_STACK_CLASS nsCSSGridTemplateAreaToken {
--- a/testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
+++ b/testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
@@ -223,22 +223,16 @@
     expected: FAIL
 
   [hsla(calc(050 + 050), 100%, 100%, 001) (standards)]
     expected: FAIL
 
   [hsla(calc(050 + 050), 100%, 100%, 001) (SVG)]
     expected: FAIL
 
-  [rgb(/**/255, 255, 255) (SVG)]
-    expected: FAIL
-
-  [rgb(/**/255/**/, /**/255/**/, /**/255/**/) (SVG)]
-    expected: FAIL
-
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (quirks)]
     expected: FAIL
 
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (almost standards)]
     expected: FAIL
 
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (standards)]
     expected: FAIL