Fixing inconsistency with Media Query append/deleteMedium.

Source/core/css/MediaList.cpp

 /*
  * (C) 1999-2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2004, 2006, 2010, 2012 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 57 matching lines @@
 MediaQuerySet::MediaQuerySet()
     : m_fallbackToDescriptor(false)
     , m_lastLine(0)
 {
 }
 
 MediaQuerySet::MediaQuerySet(const String& mediaString, bool fallbackToDescriptor)
     : m_fallbackToDescriptor(fallbackToDescriptor)
     , m_lastLine(0)
 {
-    bool success = parse(mediaString);
-    // FIXME: parsing can fail. The problem with failing constructor is that
-    // we would need additional flag saying MediaList is not valid
-    // Parse can fail only when fallbackToDescriptor == false, i.e when HTML4 media descriptor
-    // forward-compatible syntax is not in use.
-    // DOMImplementationCSS seems to mandate that media descriptors are used
-    // for both html and svg, even though svg:style doesn't use media descriptors
-    // Currently the only places where parsing can fail are
-    // creating <svg:style>, creating css media / import rules from js
-
-    // FIXME: This doesn't make much sense.
-    if (!success)
-        parse("invalid");
+    set(mediaString);
 }
 
 MediaQuerySet::MediaQuerySet(const MediaQuerySet& o)
     : RefCounted<MediaQuerySet>()
     , m_fallbackToDescriptor(o.m_fallbackToDescriptor)
     , m_lastLine(o.m_lastLine)
     , m_queries(o.m_queries.size())
 {
     for (unsigned i = 0; i < m_queries.size(); ++i)
         m_queries[i] = o.m_queries[i]->copy();
@@ skipping 29 matching lines @@
 
     if (m_fallbackToDescriptor) {
         String medium = parseMediaDescriptor(queryString);
         if (!medium.isNull())
             return adoptPtr(new MediaQuery(MediaQuery::None, medium, nullptr));
     }
 
     return adoptPtr(new MediaQuery(MediaQuery::None, "not all", nullptr));
 }
 
-bool MediaQuerySet::parse(const String& mediaString)
+void MediaQuerySet::parseMediaQueryList(const String& mediaString, Vector<OwnPtr<MediaQuery> >& result)
 {
     if (mediaString.isEmpty()) {
-        m_queries.clear();
-        return true;
+        result.clear();
+        return;
     }
 
     Vector<String> list;
     // FIXME: This is too simple as it shouldn't split when the ',' is inside
     // other allowed matching pairs such as (), [], {}, "", and ''.
     mediaString.split(',', /* allowEmptyEntries */ true, list);
 
-    Vector<OwnPtr<MediaQuery> > result;
     result.reserveInitialCapacity(list.size());
 
     for (unsigned i = 0; i < list.size(); ++i) {
         String queryString = list[i].stripWhiteSpace();
-        if (OwnPtr<MediaQuery> parsedQuery = parseMediaQuery(queryString))
-            result.uncheckedAppend(parsedQuery.release());
+        OwnPtr<MediaQuery> parsedQuery = parseMediaQuery(queryString);
+        ASSERT(parsedQuery);
+        result.uncheckedAppend(parsedQuery.release());
     }
+}
 
+bool MediaQuerySet::set(const String& mediaString)
+{
+    Vector<OwnPtr<MediaQuery> > result;
+    parseMediaQueryList(mediaString, result);
     m_queries.swap(result);
     return true;
 }
 
 bool MediaQuerySet::add(const String& queryString)
 {
-    if (OwnPtr<MediaQuery> parsedQuery = parseMediaQuery(queryString)) {
-        m_queries.append(parsedQuery.release());
-        return true;
+    // To parse a media query for a given string means to follow the parse
+    // a media query list steps and return null if more than one media query
+    // is returned or a media query if a single media query is returned.
+    Vector<OwnPtr<MediaQuery> > queries;
+    parseMediaQueryList(queryString, queries);
+
+    // If "null", terminate these steps.
+    if (queries.size() != 1)
+        return false;
+
+    OwnPtr<MediaQuery> newQuery = queries[0].release();
+    ASSERT(newQuery);
+
+    // If comparing with any of the media queries in the collection of media
+    // queries returns true terminate these steps.
+    for (size_t i = 0; i < m_queries.size(); ++i) {
+        MediaQuery* query = m_queries[i].get();
+        if (*query == *newQuery)
+            return true;
     }
-    return false;
+
+    m_queries.append(newQuery.release());
+    return true;
 }
 
 bool MediaQuerySet::remove(const String& queryStringToRemove)
 {
-    OwnPtr<MediaQuery> parsedQuery = parseMediaQuery(queryStringToRemove);
-    if (!parsedQuery)
+    // To parse a media query for a given string means to follow the parse
+    // a media query list steps and return null if more than one media query
+    // is returned or a media query if a single media query is returned.
+    Vector<OwnPtr<MediaQuery> > queries;
+    parseMediaQueryList(queryStringToRemove, queries);
+
+    // If "null", terminate these steps.
+    if (queries.size() != 1)
         return false;
 
+    OwnPtr<MediaQuery> newQuery = queries[0].release();
+    ASSERT(newQuery);
+
+    // Remove any media query from the collection of media queries for which
+    // comparing the media query with m returns true.
+    bool found = false;
     for (size_t i = 0; i < m_queries.size(); ++i) {
         MediaQuery* query = m_queries[i].get();
-        if (*query == *parsedQuery) {
+        if (*query == *newQuery) {
             m_queries.remove(i);
-            return true;
+            --i;
+            found = true;
         }
     }
-    return false;
+
+    return found;
 }
 
 void MediaQuerySet::addMediaQuery(PassOwnPtr<MediaQuery> mediaQuery)
 {
     m_queries.append(mediaQuery);
 }
 
 String MediaQuerySet::mediaText() const
 {
     StringBuilder text;
@@ skipping 26 matching lines @@
     : m_mediaQueries(mediaQueries)
     , m_parentStyleSheet(0)
     , m_parentRule(parentRule)
 {
 }
 
 MediaList::~MediaList()
 {
 }
 
-void MediaList::setMediaText(const String& value, ExceptionCode& ec)
+void MediaList::setMediaText(const String& value)
 {
     CSSStyleSheet::RuleMutationScope mutationScope(m_parentRule);
 
-    bool success = m_mediaQueries->parse(value);
-    if (!success) {
-        ec = SYNTAX_ERR;
-        return;
-    }
+    m_mediaQueries->set(value);
+
     if (m_parentStyleSheet)
         m_parentStyleSheet->didMutate();
 }
 
 String MediaList::item(unsigned index) const
 {
     const Vector<OwnPtr<MediaQuery> >& queries = m_mediaQueries->queryVector();
     if (index < queries.size())
         return queries[index]->cssText();
     return String();
@@ skipping 11 matching lines @@
     if (m_parentStyleSheet)
         m_parentStyleSheet->didMutate();
 }
 
 void MediaList::appendMedium(const String& medium, ExceptionCode& ec)
 {
     CSSStyleSheet::RuleMutationScope mutationScope(m_parentRule);
 
     bool success = m_mediaQueries->add(medium);
     if (!success) {
-        // FIXME: Should this really be INVALID_CHARACTER_ERR?
         ec = INVALID_CHARACTER_ERR;
         return;
     }
+
     if (m_parentStyleSheet)
         m_parentStyleSheet->didMutate();
 }
 
 void MediaList::reattach(MediaQuerySet* mediaQueries)
 {
     ASSERT(mediaQueries);
     m_mediaQueries = mediaQueries;
 }
 
@@ skipping 61 matching lines @@
                     CSSPrimitiveValue* primitiveValue = toCSSPrimitiveValue(cssValue);
                     if (primitiveValue->isDotsPerInch() || primitiveValue->isDotsPerCentimeter())
                         addResolutionWarningMessageToConsole(document, mediaQuerySet->mediaText(), primitiveValue);
                 }
             }
         }
     }
 }
 
 }