Diff
Modified: trunk/LayoutTests/ChangeLog (209494 => 209495)
--- trunk/LayoutTests/ChangeLog 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/LayoutTests/ChangeLog 2016-12-08 00:34:02 UTC (rev 209495)
@@ -1,3 +1,13 @@
+2016-12-07 Dave Hyatt <hy...@apple.com>
+
+ [CSS Parser] Consolidate string/ident/url serialization functions
+ https://bugs.webkit.org/show_bug.cgi?id=165552
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/css/content-language-only-whitespace-expected.txt:
+ * fast/css/content-language-with-whitespace-expected.txt:
+
2016-12-07 Ryan Haddad <ryanhad...@apple.com>
Marking imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-1.html as flaky on El Capitan WK2.
Modified: trunk/LayoutTests/fast/css/content-language-only-whitespace-expected.txt (209494 => 209495)
--- trunk/LayoutTests/fast/css/content-language-only-whitespace-expected.txt 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/LayoutTests/fast/css/content-language-only-whitespace-expected.txt 2016-12-08 00:34:02 UTC (rev 209495)
@@ -1,6 +1,6 @@
Test for bug 76701: map HTTP-EQUIV content-language to -webkit-locale. This particular test tests that a content-language with whitespace-only content is ignored. This expectation may change, see bug. HTML5 decrees that the meta element be ignored in case of whitespace only content. It's unclear what other browsers do.
-FAIL languageOfNode('x') should be auto. Was ' \9\a '.
+FAIL languageOfNode('x') should be auto. Was ' \9 \a '.
PASS languageOfNode('y') is "ar"
PASS successfullyParsed is true
Modified: trunk/LayoutTests/fast/css/content-language-with-whitespace-expected.txt (209494 => 209495)
--- trunk/LayoutTests/fast/css/content-language-with-whitespace-expected.txt 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/LayoutTests/fast/css/content-language-with-whitespace-expected.txt 2016-12-08 00:34:02 UTC (rev 209495)
@@ -1,6 +1,6 @@
Test for bug 76701: map HTTP-EQUIV content-language to -webkit-locale. This particular test tests that the the pragma-set default language is set to the first sequence of non-whitespace characters of the content-language content. This expectation may change, see bug. This expectation is as per the HTML 5 spec. It appears that Firefox does not exactly do this, but trims the leading and trailing whitespace. It's unclear what IE does.
-FAIL languageOfNode('x') should be ja_JP. Was ' \a\9\9ja-JP \9 zh_CN \9\a\a\9\9'.
+FAIL languageOfNode('x') should be ja_JP. Was ' \a \9 \9 ja-JP \9 zh_CN \9 \a \a \9 \9 '.
PASS languageOfNode('y') is "ar"
PASS successfullyParsed is true
Modified: trunk/Source/WebCore/ChangeLog (209494 => 209495)
--- trunk/Source/WebCore/ChangeLog 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/ChangeLog 2016-12-08 00:34:02 UTC (rev 209495)
@@ -1,3 +1,64 @@
+2016-12-07 Dave Hyatt <hy...@apple.com>
+
+ [CSS Parser] Consolidate string/ident/url serialization functions
+ https://bugs.webkit.org/show_bug.cgi?id=165552
+
+ Reviewed by Zalan Bujtas.
+
+ Right now CSSParser has string, ident and url serialization functions
+ called quoteCSStringIfNeeded (which actually serializes both strings and
+ identifiers), as well as quoteCSSURLIfNeeded.
+
+ CSSMarkup already has serialization functions that exist outside of the
+ CSSParser and that handle serialization of strings, idents and URLs. This
+ patch eliminates the CSSParser functions and consolidates all of the
+ serialization to use CSSMarkup's functions.
+
+ Note that we are not spec-compliant at all here, and so I had to amend
+ the functions to support our non-spec-compliant serialization. The goal
+ of this patch is consolidation and not to fix our broken serialization.
+
+ Notable changes include parameterizing string serialization so that
+ both single and double quotes are supported, since in the existing code
+ we're sometimes spec-compliant (CSSSelectors) and sometimes not
+ (CSSPrimitiveValue).
+
+ We also overload CSS_STRING primitive value type and have it act as both
+ a string and a custom identifier. This is lame, since the parser should
+ have made two different types of objects instead, but since our parser
+ doesn't do that yet, I added a serializeAsStringOrCustomIdent that
+ preserves our old behavior of "quote the string only if needed." In this
+ case what that really meant was "Try to guess that we were originally a
+ custom ident and leave off quotes if so." This function will go away
+ once we properly create CSSStringValues and CSSCustomIdentValues instead
+ of turning the latter into strings.
+
+ * css/CSSBasicShapes.cpp:
+ (WebCore::buildPathString):
+ * css/CSSImageValue.cpp:
+ (WebCore::CSSImageValue::customCSSText):
+ * css/CSSMarkup.cpp:
+ (WebCore::isCSSTokenizerURL):
+ (WebCore::serializeString):
+ (WebCore::serializeURL):
+ (WebCore::serializeAsStringOrCustomIdent):
+ (WebCore::serializeURI): Deleted.
+ * css/CSSMarkup.h:
+ * css/CSSPrimitiveValue.cpp:
+ (WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText):
+ * css/CSSSelector.cpp:
+ (WebCore::CSSSelector::selectorText):
+ * css/parser/CSSParser.cpp:
+ (WebCore::isCSSTokenizerIdent): Deleted.
+ (WebCore::isCSSTokenizerURL): Deleted.
+ (WebCore::quoteCSSStringInternal): Deleted.
+ (WebCore::quoteCSSString): Deleted.
+ (WebCore::quoteCSSStringIfNeeded): Deleted.
+ (WebCore::quoteCSSURLIfNeeded): Deleted.
+ * css/parser/CSSParser.h:
+ * html/HTMLElement.cpp:
+ (WebCore::HTMLElement::mapLanguageAttributeToLocale):
+
2016-12-07 Dean Jackson <d...@apple.com>
Expose internal API to detect media documents
Modified: trunk/Source/WebCore/css/CSSBasicShapes.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/CSSBasicShapes.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSBasicShapes.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -31,7 +31,7 @@
#include "CSSBasicShapes.h"
-#include "CSSParser.h"
+#include "CSSMarkup.h"
#include "CSSPrimitiveValueMappings.h"
#include "CSSValuePool.h"
#include "Pair.h"
@@ -216,7 +216,7 @@
else
result.appendLiteral("path(");
- result.append(quoteCSSString(path));
+ serializeString(path, result);
result.append(')');
if (box.length()) {
Modified: trunk/Source/WebCore/css/CSSImageValue.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/CSSImageValue.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSImageValue.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -22,7 +22,8 @@
#include "CSSImageValue.h"
#include "CSSCursorImageValue.h"
-#include "CSSParser.h"
+#include "CSSMarkup.h"
+#include "CSSPrimitiveValue.h"
#include "CSSValueKeywords.h"
#include "CachedImage.h"
#include "CachedResourceLoader.h"
@@ -94,7 +95,7 @@
String CSSImageValue::customCSSText() const
{
- return "url(" + quoteCSSURLIfNeeded(m_url) + ')';
+ return serializeURL(m_url);
}
Ref<CSSValue> CSSImageValue::cloneForCSSOM() const
Modified: trunk/Source/WebCore/css/CSSMarkup.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/CSSMarkup.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSMarkup.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -118,10 +118,54 @@
}
}
-void serializeString(const String& string, StringBuilder& appendTo)
+template <typename CharacterType>
+static inline bool isCSSTokenizerURL(const CharacterType* characters, unsigned length)
{
- appendTo.append('\"');
+ const CharacterType* end = characters + length;
+
+ for (; characters != end; ++characters) {
+ CharacterType c = characters[0];
+ switch (c) {
+ case '!':
+ case '#':
+ case '$':
+ case '%':
+ case '&':
+ break;
+ default:
+ if (c < '*')
+ return false;
+ if (c <= '~')
+ break;
+ if (c < 128)
+ return false;
+ }
+ }
+
+ return true;
+}
+// "url" from the CSS tokenizer, minus backslash-escape sequences
+static bool isCSSTokenizerURL(const String& string)
+{
+ unsigned length = string.length();
+
+ if (!length)
+ return true;
+
+ if (string.is8Bit())
+ return isCSSTokenizerURL(string.characters8(), length);
+ return isCSSTokenizerURL(string.characters16(), length);
+}
+
+void serializeString(const String& string, StringBuilder& appendTo, bool useDoubleQuotes)
+{
+ // FIXME: From the CSS OM draft:
+ // To serialize a string means to create a string represented by '"' (U+0022).
+ // We need to switch to using " instead of ', but this involves patching a large
+ // number of tests and changing editing code to not get confused by double quotes.
+ appendTo.append(useDoubleQuotes ? '\"' : '\'');
+
unsigned index = 0;
while (index < string.length()) {
UChar32 c = string.characterStartingAt(index);
@@ -135,21 +179,36 @@
appendTo.append(c);
}
- appendTo.append('\"');
+ appendTo.append(useDoubleQuotes ? '\"' : '\'');
}
-String serializeString(const String& string)
+String serializeString(const String& string, bool useDoubleQuotes)
{
StringBuilder builder;
- serializeString(string, builder);
+ serializeString(string, builder, useDoubleQuotes);
return builder.toString();
}
-String serializeURI(const String& string)
+String serializeURL(const String& string)
{
- return "url(" + serializeString(string) + ")";
+ // FIXME: URLS must always be double quoted. From the CSS OM draft:
+ // To serialize a URL means to create a string represented by "url(", followed by
+ // the serialization of the URL as a string, followed by ")".
+ // To keep backwards compatibility with existing tests, for now we only quote if needed and
+ // we use a single quote.
+ return "url(" + (isCSSTokenizerURL(string) ? string : serializeString(string)) + ")";
}
+String serializeAsStringOrCustomIdent(const String& string)
+{
+ if (isCSSTokenizerIdentifier(string)) {
+ StringBuilder builder;
+ serializeIdentifier(string, builder);
+ return builder.toString();
+ }
+ return serializeString(string);
+}
+
String serializeFontFamily(const String& string)
{
return isCSSTokenizerIdentifier(string) ? string : serializeString(string);
Modified: trunk/Source/WebCore/css/CSSMarkup.h (209494 => 209495)
--- trunk/Source/WebCore/css/CSSMarkup.h 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSMarkup.h 2016-12-08 00:34:02 UTC (rev 209495)
@@ -30,9 +30,13 @@
// Common serializing methods. See: http://dev.w3.org/csswg/cssom/#common-serializing-idioms
void serializeIdentifier(const String& identifier, StringBuilder& appendTo, bool skipStartChecks = false);
-void serializeString(const String&, StringBuilder& appendTo);
-String serializeString(const String&);
-String serializeURI(const String&);
+void serializeString(const String&, StringBuilder& appendTo, bool useDoubleQuotes = false);
+String serializeString(const String&, bool useDoubleQuotes = false);
+String serializeURL(const String&);
String serializeFontFamily(const String&);
+// FIXME-NEWPARSER: This hybrid "check for both string or ident" function can be removed
+// once we have enabled CSSCustomIdentValue and CSSStringValue.
+String serializeAsStringOrCustomIdent(const String&);
+
} // namespace WebCore
Modified: trunk/Source/WebCore/css/CSSPrimitiveValue.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/CSSPrimitiveValue.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSPrimitiveValue.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -25,7 +25,8 @@
#include "CSSCalculationValue.h"
#include "CSSFontFamily.h"
#include "CSSHelper.h"
-#include "CSSParser.h"
+#include "CSSMarkup.h"
+#include "CSSParserValues.h"
#include "CSSPrimitiveValueMappings.h"
#include "CSSPropertyNames.h"
#include "CSSToLengthConversionData.h"
@@ -1068,11 +1069,14 @@
// FIXME: We currently don't handle CSS_DIMENSION properly as we don't store
// the actual dimension, just the numeric value as a string.
case CSS_STRING:
- return quoteCSSStringIfNeeded(m_value.string);
+ // FIME-NEWPARSER: Once we have CSSCustomIdentValue hooked up, this can just be
+ // serializeString, since custom identifiers won't be the same value as strings
+ // any longer.
+ return serializeAsStringOrCustomIdent(m_value.string);
case CSS_FONT_FAMILY:
- return quoteCSSStringIfNeeded(m_value.fontFamily->familyName);
+ return serializeFontFamily(m_value.fontFamily->familyName);
case CSS_URI:
- return "url(" + quoteCSSURLIfNeeded(m_value.string) + ')';
+ return serializeURL(m_value.string);
case CSS_VALUE_ID:
return valueName(m_value.valueID);
case CSS_PROPERTY_ID:
@@ -1099,7 +1103,7 @@
result.append(m_value.counter->identifier());
if (!separator.isEmpty()) {
result.appendLiteral(", ");
- result.append(quoteCSSStringIfNeeded(separator));
+ serializeString(separator, result);
}
String listStyle = m_value.counter->listStyle();
if (!listStyle.isEmpty()) {
Modified: trunk/Source/WebCore/css/CSSSelector.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/CSSSelector.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/CSSSelector.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -697,7 +697,7 @@
break;
}
if (cs->match() != CSSSelector::Set) {
- serializeString(cs->serializingValue(), str);
+ serializeString(cs->serializingValue(), str, true);
if (cs->attributeValueMatchingIsCaseInsensitive())
str.appendLiteral(" i]");
else
Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (209494 => 209495)
--- trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -13699,161 +13699,6 @@
return string.is8Bit() ? cssValueKeywordID(string.characters8(), length) : cssValueKeywordID(string.characters16(), length);
}
-template <typename CharacterType>
-static inline bool isCSSTokenizerIdent(const CharacterType* characters, unsigned length)
-{
- const CharacterType* end = characters + length;
-
- // -?
- if (characters != end && characters[0] == '-')
- ++characters;
-
- // {nmstart}
- if (characters == end || !(characters[0] == '_' || characters[0] >= 128 || isASCIIAlpha(characters[0])))
- return false;
- ++characters;
-
- // {nmchar}*
- for (; characters != end; ++characters) {
- if (!(characters[0] == '_' || characters[0] == '-' || characters[0] >= 128 || isASCIIAlphanumeric(characters[0])))
- return false;
- }
-
- return true;
-}
-
-// "ident" from the CSS tokenizer, minus backslash-escape sequences
-static bool isCSSTokenizerIdent(const String& string)
-{
- unsigned length = string.length();
-
- if (!length)
- return false;
-
- if (string.is8Bit())
- return isCSSTokenizerIdent(string.characters8(), length);
- return isCSSTokenizerIdent(string.characters16(), length);
-}
-
-template <typename CharacterType>
-static inline bool isCSSTokenizerURL(const CharacterType* characters, unsigned length)
-{
- const CharacterType* end = characters + length;
-
- for (; characters != end; ++characters) {
- CharacterType c = characters[0];
- switch (c) {
- case '!':
- case '#':
- case '$':
- case '%':
- case '&':
- break;
- default:
- if (c < '*')
- return false;
- if (c <= '~')
- break;
- if (c < 128)
- return false;
- }
- }
-
- return true;
-}
-
-// "url" from the CSS tokenizer, minus backslash-escape sequences
-static bool isCSSTokenizerURL(const String& string)
-{
- unsigned length = string.length();
-
- if (!length)
- return true;
-
- if (string.is8Bit())
- return isCSSTokenizerURL(string.characters8(), length);
- return isCSSTokenizerURL(string.characters16(), length);
-}
-
-
-template <typename CharacterType>
-static inline String quoteCSSStringInternal(const CharacterType* characters, unsigned length)
-{
- // For efficiency, we first pre-calculate the length of the quoted string, then we build the actual one.
- // Please see below for the actual logic.
- unsigned quotedStringSize = 2; // Two quotes surrounding the entire string.
- bool afterEscape = false;
- for (unsigned i = 0; i < length; ++i) {
- CharacterType ch = characters[i];
- if (ch == '\\' || ch == '\'') {
- quotedStringSize += 2;
- afterEscape = false;
- } else if (ch < 0x20 || ch == 0x7F) {
- quotedStringSize += 2 + (ch >= 0x10);
- afterEscape = true;
- } else {
- quotedStringSize += 1 + (afterEscape && (isASCIIHexDigit(ch) || ch == ' '));
- afterEscape = false;
- }
- }
-
- StringBuffer<CharacterType> buffer(quotedStringSize);
- unsigned index = 0;
- buffer[index++] = '\'';
- afterEscape = false;
- for (unsigned i = 0; i < length; ++i) {
- CharacterType ch = characters[i];
- if (ch == '\\' || ch == '\'') {
- buffer[index++] = '\\';
- buffer[index++] = ch;
- afterEscape = false;
- } else if (ch < 0x20 || ch == 0x7F) { // Control characters.
- buffer[index++] = '\\';
- placeByteAsHexCompressIfPossible(ch, buffer, index, Lowercase);
- afterEscape = true;
- } else {
- // Space character may be required to separate backslash-escape sequence and normal characters.
- if (afterEscape && (isASCIIHexDigit(ch) || ch == ' '))
- buffer[index++] = ' ';
- buffer[index++] = ch;
- afterEscape = false;
- }
- }
- buffer[index++] = '\'';
-
- ASSERT(quotedStringSize == index);
- return String::adopt(WTFMove(buffer));
-}
-
-// We use single quotes for now because markup.cpp uses double quotes.
-String quoteCSSString(const String& string)
-{
- // This function expands each character to at most 3 characters ('\u0010' -> '\' '1' '0') as well as adds
- // 2 quote characters (before and after). Make sure the resulting size (3 * length + 2) will not overflow unsigned.
-
- unsigned length = string.length();
-
- if (!length)
- return ASCIILiteral("\'\'");
-
- if (length > std::numeric_limits<unsigned>::max() / 3 - 2)
- return emptyString();
-
- if (string.is8Bit())
- return quoteCSSStringInternal(string.characters8(), length);
- return quoteCSSStringInternal(string.characters16(), length);
-}
-
-String quoteCSSStringIfNeeded(const String& string)
-{
- return isCSSTokenizerIdent(string) ? string : quoteCSSString(string);
-}
-
-String quoteCSSURLIfNeeded(const String& string)
-{
- return isCSSTokenizerURL(string) ? string : quoteCSSString(string);
-}
-
bool isValidNthToken(const CSSParserString& token)
{
// The tokenizer checks for the construct of an+b.
Modified: trunk/Source/WebCore/css/parser/CSSParser.h (209494 => 209495)
--- trunk/Source/WebCore/css/parser/CSSParser.h 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/css/parser/CSSParser.h 2016-12-08 00:34:02 UTC (rev 209495)
@@ -753,10 +753,6 @@
CSSParserString token;
};
-String quoteCSSString(const String&);
-String quoteCSSStringIfNeeded(const String&);
-String quoteCSSURLIfNeeded(const String&);
-
bool isValidNthToken(const CSSParserString&);
template <>
Modified: trunk/Source/WebCore/html/HTMLElement.cpp (209494 => 209495)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2016-12-08 00:27:35 UTC (rev 209494)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2016-12-08 00:34:02 UTC (rev 209495)
@@ -25,7 +25,7 @@
#include "config.h"
#include "HTMLElement.h"
-#include "CSSParser.h"
+#include "CSSMarkup.h"
#include "CSSPropertyNames.h"
#include "CSSValueKeywords.h"
#include "CSSValuePool.h"
@@ -117,7 +117,7 @@
{
if (!value.isEmpty()) {
// Have to quote so the locale id is treated as a string instead of as a CSS keyword.
- addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitLocale, quoteCSSString(value));
+ addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitLocale, serializeString(value));
} else {
// The empty string means the language is explicitly unknown.
addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitLocale, CSSValueAuto);