Improve warnings for non-standard meta tag separators

third_party/WebKit/Source/core/html/HTMLMetaElement-in.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2010 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.
@@ skipping 55 matching lines @@
 }
 
 // Though isspace() considers \t and \v to be whitespace, Win IE doesn't.
 static bool isSeparator(UChar c)
 {
     return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == ',' || c == '\0';
 }
 
 void HTMLMetaElement::parseContentAttribute(const String& content, void* data, Document* document, bool viewportMetaZeroValuesQuirk)
 {
-    bool error = false;
+    bool hasInvalidSeparator = false;
 
     // Tread lightly in this code -- it was specifically designed to mimic Win IE's parsing behavior.
     unsigned keyBegin, keyEnd;
     unsigned valueBegin, valueEnd;
 
     String buffer = content.lower();
     unsigned length = buffer.length();
     for (unsigned i = 0; i < length; /* no increment here */) {
         // skip to first non-separator, but don't skip past the end of the string
         while (isSeparator(buffer[i])) {
             if (i >= length)
                 break;
             i++;
         }
         keyBegin = i;
 
         // skip to first separator
         while (!isSeparator(buffer[i])) {
-            error |= isInvalidSeparator(buffer[i]);
+            hasInvalidSeparator |= isInvalidSeparator(buffer[i]);
             if (i >= length)
                 break;
             i++;
         }
         keyEnd = i;
 
         // skip to first '=', but don't skip past a ',' or the end of the string
         while (buffer[i] != '=') {
-            error |= isInvalidSeparator(buffer[i]);
+            hasInvalidSeparator |= isInvalidSeparator(buffer[i]);
             if (buffer[i] == ',' || i >= length)
                 break;
             i++;
         }
 
         // skip to first non-separator, but don't skip past a ',' or the end of the string
         while (isSeparator(buffer[i])) {
             if (buffer[i] == ',' || i >= length)
                 break;
             i++;
         }
         valueBegin = i;
 
         // skip to first separator
         while (!isSeparator(buffer[i])) {
-            error |= isInvalidSeparator(buffer[i]);
+            hasInvalidSeparator |= isInvalidSeparator(buffer[i]);
             if (i >= length)
                 break;
             i++;
         }
         valueEnd = i;
 
         SECURITY_DCHECK(i <= length);
 
         String keyString = buffer.substring(keyBegin, keyEnd - keyBegin);
         String valueString = buffer.substring(valueBegin, valueEnd - valueBegin);
-        processViewportKeyValuePair(document, keyString, valueString, viewportMetaZeroValuesQuirk, data);
+        processViewportKeyValuePair(document, hasInvalidSeparator, keyString, valueString, viewportMetaZeroValuesQuirk, data);
     }
-    if (error && document) {
+    if (hasInvalidSeparator && document) {
         String message = "Error parsing a meta element's content: ';' is not a valid key-value pair separator. Please use ',' instead.";
         document->addConsoleMessage(ConsoleMessage::create(RenderingMessageSource, WarningMessageLevel, message));
     }
 }
 
 static inline float clampLengthValue(float value)
 {
     // Limits as defined in the css-device-adapt spec.
     if (value != ViewportDescription::ValueAuto)
         return std::min(float(10000), std::max(value, float(1)));
     return value;
 }
 
 static inline float clampScaleValue(float value)
 {
     // Limits as defined in the css-device-adapt spec.
     if (value != ViewportDescription::ValueAuto)
         return std::min(float(10), std::max(value, float(0.1)));
     return value;
 }
 
-float HTMLMetaElement::parsePositiveNumber(Document* document, const String& keyString, const String& valueString, bool* ok)
+float HTMLMetaElement::parsePositiveNumber(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString, bool* ok)

[bokan, 2016/09/07 15:13:06]

In this method and others, lets call the parameter `reportWarnings`, rather than
`hasInvalidSeparator`, and make its meaning opposite.

The variable name in the above method is fine though, that is, from
parseContentAttribute we keep track of hasInvalidSeparator and call
processViewportKeyValuePair which takes `reportWarnings` for which we pass
!hasInvalidSeparator.

 {
     size_t parsedLength;
     float value;
     if (valueString.is8Bit())
         value = charactersToFloat(valueString.characters8(), valueString.length(), parsedLength);
     else
         value = charactersToFloat(valueString.characters16(), valueString.length(), parsedLength);
     if (!parsedLength) {
-        reportViewportWarning(document, UnrecognizedViewportArgumentValueError, valueString, keyString);
+        reportViewportWarning(document, hasInvalidSeparator, UnrecognizedViewportArgumentValueError, valueString, keyString);
         if (ok)
             *ok = false;
         return 0;
     }
     if (parsedLength < valueString.length())
-        reportViewportWarning(document, TruncatedViewportArgumentValueError, valueString, keyString);
+        reportViewportWarning(document, hasInvalidSeparator, TruncatedViewportArgumentValueError, valueString, keyString);
     if (ok)
         *ok = true;
     return value;
 }
 
-Length HTMLMetaElement::parseViewportValueAsLength(Document* document, const String& keyString, const String& valueString)
+Length HTMLMetaElement::parseViewportValueAsLength(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString)
 {
     // 1) Non-negative number values are translated to px lengths.
     // 2) Negative number values are translated to auto.
     // 3) device-width and device-height are used as keywords.
     // 4) Other keywords and unknown values translate to 0.0.
 
     unsigned length = valueString.length();
     DEFINE_ARRAY_FOR_MATCHING(characters, valueString, 13);
     SWITCH(characters, length) {
         CASE("device-width") {
             return Length(DeviceWidth);
         }
         CASE("device-height") {
             return Length(DeviceHeight);
         }
     }
 
-    float value = parsePositiveNumber(document, keyString, valueString);
+    float value = parsePositiveNumber(document, hasInvalidSeparator, keyString, valueString);
 
     if (value < 0)
         return Length(); // auto
 
     return Length(clampLengthValue(value), Fixed);
 }
 
-float HTMLMetaElement::parseViewportValueAsZoom(Document* document, const String& keyString, const String& valueString, bool& computedValueMatchesParsedValue, bool viewportMetaZeroValuesQuirk)
+float HTMLMetaElement::parseViewportValueAsZoom(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString, bool& computedValueMatchesParsedValue, bool viewportMetaZeroValuesQuirk)
 {
     // 1) Non-negative number values are translated to <number> values.
     // 2) Negative number values are translated to auto.
     // 3) yes is translated to 1.0.
     // 4) device-width and device-height are translated to 10.0.
     // 5) no and unknown values are translated to 0.0
 
     computedValueMatchesParsedValue = false;
     unsigned length = valueString.length();
     DEFINE_ARRAY_FOR_MATCHING(characters, valueString, 13);
     SWITCH(characters, length) {
         CASE("yes") {
             return 1;
         }
         CASE("no") {
             return 0;
         }
         CASE("device-width") {
             return 10;
         }
         CASE("device-height") {
             return 10;
         }
     }
 
-    float value = parsePositiveNumber(document, keyString, valueString);
+    float value = parsePositiveNumber(document, hasInvalidSeparator, keyString, valueString);
 
     if (value < 0)
         return ViewportDescription::ValueAuto;
 
     if (value > 10.0)
-        reportViewportWarning(document, MaximumScaleTooLargeError, String(), String());
+        reportViewportWarning(document, hasInvalidSeparator, MaximumScaleTooLargeError, String(), String());
 
     if (!value && viewportMetaZeroValuesQuirk)
         return ViewportDescription::ValueAuto;
 
     float clampedValue = clampScaleValue(value);
     if (clampedValue == value)
         computedValueMatchesParsedValue = true;
 
     return clampedValue;
 }
 
-bool HTMLMetaElement::parseViewportValueAsUserZoom(Document* document, const String& keyString, const String& valueString, bool& computedValueMatchesParsedValue)
+bool HTMLMetaElement::parseViewportValueAsUserZoom(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString, bool& computedValueMatchesParsedValue)
 {
     // yes and no are used as keywords.
     // Numbers >= 1, numbers <= -1, device-width and device-height are mapped to yes.
     // Numbers in the range <-1, 1>, and unknown values, are mapped to no.
 
     computedValueMatchesParsedValue = false;
     unsigned length = valueString.length();
     DEFINE_ARRAY_FOR_MATCHING(characters, valueString, 13);
     SWITCH(characters, length) {
         CASE("yes") {
             computedValueMatchesParsedValue = true;
             return true;
         }
         CASE("no") {
             computedValueMatchesParsedValue = true;
             return false;
         }
         CASE("device-width") {
             return true;
         }
         CASE("device-height") {
             return true;
         }
     }
 
-    float value = parsePositiveNumber(document, keyString, valueString);
+    float value = parsePositiveNumber(document, hasInvalidSeparator, keyString, valueString);
     if (fabs(value) < 1)
         return false;
 
     return true;
 }
 
-float HTMLMetaElement::parseViewportValueAsDPI(Document* document, const String& keyString, const String& valueString)
+float HTMLMetaElement::parseViewportValueAsDPI(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString)
 {
     unsigned length = valueString.length();
     DEFINE_ARRAY_FOR_MATCHING(characters, valueString, 10);
     SWITCH(characters, length) {
         CASE("device-dpi") {
             return ViewportDescription::ValueDeviceDPI;
         }
         CASE("low-dpi") {
             return ViewportDescription::ValueLowDPI;
         }
         CASE("medium-dpi") {
             return ViewportDescription::ValueMediumDPI;
         }
         CASE("high-dpi") {
             return ViewportDescription::ValueHighDPI;
         }
     }
 
     bool ok;
-    float value = parsePositiveNumber(document, keyString, valueString, &ok);
+    float value = parsePositiveNumber(document, hasInvalidSeparator, keyString, valueString, &ok);
     if (!ok || value < 70 || value > 400)
         return ViewportDescription::ValueAuto;
 
     return value;
 }
 
-void HTMLMetaElement::processViewportKeyValuePair(Document* document, const String& keyString, const String& valueString, bool viewportMetaZeroValuesQuirk, void* data)
+void HTMLMetaElement::processViewportKeyValuePair(Document* document, bool& hasInvalidSeparator, const String& keyString, const String& valueString, bool viewportMetaZeroValuesQuirk, void* data)
 {
     ViewportDescription* description = static_cast<ViewportDescription*>(data);
 
     unsigned length = keyString.length();
 
     DEFINE_ARRAY_FOR_MATCHING(characters, keyString, 17);
     SWITCH(characters, length) {
         CASE("width") {
-            const Length& width = parseViewportValueAsLength(document, keyString, valueString);
+            const Length& width = parseViewportValueAsLength(document, hasInvalidSeparator, keyString, valueString);
             if (width.isAuto())
                 return;
             description->minWidth = Length(ExtendToZoom);
             description->maxWidth = width;
             return;
         }
         CASE("height") {
-            const Length& height = parseViewportValueAsLength(document, keyString, valueString);
+            const Length& height = parseViewportValueAsLength(document, hasInvalidSeparator, keyString, valueString);
             if (height.isAuto())
                 return;
             description->minHeight = Length(ExtendToZoom);
             description->maxHeight = height;
             return;
         }
         CASE("initial-scale") {
-            description->zoom = parseViewportValueAsZoom(document, keyString, valueString, description->zoomIsExplicit, viewportMetaZeroValuesQuirk);
+            description->zoom = parseViewportValueAsZoom(document, hasInvalidSeparator, keyString, valueString, description->zoomIsExplicit, viewportMetaZeroValuesQuirk);
             return;
         }
         CASE("minimum-scale") {
-            description->minZoom = parseViewportValueAsZoom(document, keyString, valueString, description->minZoomIsExplicit, viewportMetaZeroValuesQuirk);
+            description->minZoom = parseViewportValueAsZoom(document, hasInvalidSeparator, keyString, valueString, description->minZoomIsExplicit, viewportMetaZeroValuesQuirk);
             return;
         }
         CASE("maximum-scale") {
-            description->maxZoom = parseViewportValueAsZoom(document, keyString, valueString, description->maxZoomIsExplicit, viewportMetaZeroValuesQuirk);
+            description->maxZoom = parseViewportValueAsZoom(document, hasInvalidSeparator, keyString, valueString, description->maxZoomIsExplicit, viewportMetaZeroValuesQuirk);
             return;
         }
         CASE("user-scalable") {
-            description->userZoom = parseViewportValueAsUserZoom(document, keyString, valueString, description->userZoomIsExplicit);
+            description->userZoom = parseViewportValueAsUserZoom(document, hasInvalidSeparator, keyString, valueString, description->userZoomIsExplicit);
             return;
         }
         CASE("target-densitydpi") {
-            description->deprecatedTargetDensityDPI = parseViewportValueAsDPI(document, keyString, valueString);
-            reportViewportWarning(document, TargetDensityDpiUnsupported, String(), String());
+            description->deprecatedTargetDensityDPI = parseViewportValueAsDPI(document, hasInvalidSeparator, keyString, valueString);
+            reportViewportWarning(document, hasInvalidSeparator, TargetDensityDpiUnsupported, String(), String());
             return;
         }
         CASE("minimal-ui") {
             // Ignore vendor-specific argument.
             return;
         }
         CASE("shrink-to-fit") {
             // Ignore vendor-specific argument.
             return;
         }
     }
-    reportViewportWarning(document, UnrecognizedViewportArgumentKeyError, keyString, String());
+    reportViewportWarning(document, hasInvalidSeparator, UnrecognizedViewportArgumentKeyError, keyString, String());
 }
 
 static const char* viewportErrorMessageTemplate(ViewportErrorCode errorCode)
 {
     static const char* const errors[] = {
         "The key \"%replacement1\" is not recognized and ignored.",
         "The value \"%replacement1\" for key \"%replacement2\" is invalid, and has been ignored.",
         "The value \"%replacement1\" for key \"%replacement2\" was truncated to its numeric prefix.",
         "The value for key \"maximum-scale\" is out of bounds and the value has been clamped.",
         "The key \"target-densitydpi\" is not supported.",
@@ skipping 10 matching lines @@
     case UnrecognizedViewportArgumentKeyError:
     case UnrecognizedViewportArgumentValueError:
     case MaximumScaleTooLargeError:
         return WarningMessageLevel;
     }
 
     NOTREACHED();
     return ErrorMessageLevel;
 }
 
-void HTMLMetaElement::reportViewportWarning(Document* document, ViewportErrorCode errorCode, const String& replacement1, const String& replacement2)
+void HTMLMetaElement::reportViewportWarning(Document* document, bool& hasInvalidSeparator, ViewportErrorCode errorCode, const String& replacement1, const String& replacement2)

[bokan, 2016/09/07 15:13:06]

This method shouldn't have this param. Instead, the caller should avoid calling
it based on `reportWarnings`

 {
-    if (!document || !document->frame())
+    if (!document || !document->frame() || hasInvalidSeparator)
         return;
 
     String message = viewportErrorMessageTemplate(errorCode);
     if (!replacement1.isNull())
         message.replace("%replacement1", replacement1);
     if (!replacement2.isNull())
         message.replace("%replacement2", replacement2);
 
     // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
     document->addConsoleMessage(ConsoleMessage::create(RenderingMessageSource, viewportErrorMessageLevel(errorCode), message));
@@ skipping 110 matching lines @@
 {
     return getAttribute(http_equivAttr);
 }
 
 const AtomicString& HTMLMetaElement::name() const
 {
     return getNameAttribute();
 }
 
 }