Changeset 291712 in webkit

Timestamp:

Mar 22, 2022, 3:22:09 PM (5 years ago)

Author:

Patrick Angle

Message:

Source/WebInspectorUI:
Web Inspector: console.log format strings containing invalid specifiers results in [Object object] replacing the specifier instead of ignoring the invalid specifier
​https://bugs.webkit.org/show_bug.cgi?id=235061

Reviewed by Devin Rousso.

Logged console messages previously allows an specifier, even invalid ones, to be replaced with values. We should
instead ignore the invalid specifier to match Firefox's behavior. (Chrome's behavior currently is to remove the
invalid specifier from the result without replacing it with the first next available substitution value.)

Other call sites using String.format are handling localized strings or other backend-provided strings that
should always be well-formatted, so falling back to handling a format specifier as a string is not a particularly
useful behavior.

• UserInterface/Base/Utilities.js:

(value):

LayoutTests:
Web Inspector: console.log print strings contains '%_' result is Object
​https://bugs.webkit.org/show_bug.cgi?id=235061

Reviewed by Devin Rousso.

• inspector/unit-tests/string-utilities-expected.txt:
• inspector/unit-tests/string-utilities.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/unit-tests/string-utilities-expected.txt (modified) (1 diff)
• LayoutTests/inspector/unit-tests/string-utilities.html (modified) (1 diff)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/Base/Utilities.js (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-03-22  Patrick Angle  <pangle@apple.com>
+
+        Web Inspector: console.log print strings contains '%_' result is Object
+        https://bugs.webkit.org/show_bug.cgi?id=235061
+
+        Reviewed by Devin Rousso.
+
+        * inspector/unit-tests/string-utilities-expected.txt:
+        * inspector/unit-tests/string-utilities.html:
+        
 2022-03-22  Matteo Flores  <matteo_flores@apple.com>
 

trunk/LayoutTests/inspector/unit-tests/string-utilities-expected.txt

@@ -20 +20 @@
 PASS: integer format specifier with Infinity argument should show "NaN"
 PASS: integer format specifier with NaN argument should show "NaN"
+Testing string with unknown specifier.
+WARN: String.format("%_ %s", "first", "second"): Unsupported format specifier "%_" will be ignored.
+PASS: Formatter string with unknown specifier should not replace unknown specifier.
+PASS: Formatter string with unknown specifier should have an unused substitution value.
 
 -- Running test case: String.prototype.extendedLocaleCompare

trunk/LayoutTests/inspector/unit-tests/string-utilities.html

@@ -31 +31 @@
             InspectorTest.expectThat("%d".format(Infinity) === "NaN", "integer format specifier with Infinity argument should show \"NaN\"");
             InspectorTest.expectThat("%d".format(NaN) === "NaN", "integer format specifier with NaN argument should show \"NaN\"");
+
+            InspectorTest.log("Testing string with unknown specifier.");
+            let formatWithUnknownSpecifiers = String.format("%_ %s", ["first", "second"], String.standardFormatters, "", function(a, b) { return a + b; });
+            InspectorTest.expectThat(formatWithUnknownSpecifiers.formattedResult === "%_ first", "Formatter string with unknown specifier should not replace unknown specifier.");
+            InspectorTest.expectThat(formatWithUnknownSpecifiers.unusedSubstitutions[0] === "second", "Formatter string with unknown specifier should have an unused substitution value.");
+
             return true;
         }

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2022-03-22  Patrick Angle  <pangle@apple.com>
+
+        Web Inspector: `console.log` format strings containing invalid specifiers results in `[Object object]` replacing the specifier instead of ignoring the invalid specifier
+        https://bugs.webkit.org/show_bug.cgi?id=235061
+
+        Reviewed by Devin Rousso.
+
+        Logged console messages previously allows an specifier, even invalid ones, to be replaced with values. We should
+        instead ignore the invalid specifier to match Firefox's behavior. (Chrome's behavior currently is to remove the
+        invalid specifier from the result without replacing it with the first next available substitution value.)
+
+        Other call sites using `String.format` are handling localized strings or other backend-provided strings that
+        should always be well-formatted, so falling back to handling a format specifier as a string is not a particularly
+        useful behavior.
+
+        * UserInterface/Base/Utilities.js:
+        (value):
+
 2022-03-22  Nikita Vasilyev  <nvasilyev@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js

@@ -1126 +1126 @@
         var tokens = String.tokenizeFormatString(format);
         var usedSubstitutionIndexes = {};
+        let ignoredUnknownSpecifierCount = 0;
 
         for (var i = 0; i < tokens.length; ++i) {
@@ -1140 +1141 @@
             }
 
-            if (token.substitutionIndex >= substitutions.length) {
+            let substitutionIndex = token.substitutionIndex - ignoredUnknownSpecifierCount;
+            if (substitutionIndex >= substitutions.length) {
                 // If there are not enough substitutions for the current substitutionIndex
                 // just output the format specifier literally and move on.
-                error("not enough substitution arguments. Had " + substitutions.length + " but needed " + (token.substitutionIndex + 1) + ", so substitution was skipped.");
+                error("not enough substitution arguments. Had " + substitutions.length + " but needed " + (substitutionIndex + 1) + ", so substitution was skipped.");
                 result = append(result, "%" + (token.precision > -1 ? token.precision : "") + token.specifier);
                 continue;
             }
 
-            usedSubstitutionIndexes[token.substitutionIndex] = true;
-
             if (!(token.specifier in formatters)) {
-                // Encountered an unsupported format character, treat as a string.
-                warn("unsupported format character \u201C" + token.specifier + "\u201D. Treating as a string.");
-                result = append(result, substitutions[token.substitutionIndex]);
+                warn(`Unsupported format specifier "%${token.specifier}" will be ignored.`);
+                result = append(result, "%" + token.specifier);
+                ++ignoredUnknownSpecifierCount;
                 continue;
             }
 
-            result = append(result, formatters[token.specifier](substitutions[token.substitutionIndex], token));
+            usedSubstitutionIndexes[substitutionIndex] = true;
+            result = append(result, formatters[token.specifier](substitutions[substitutionIndex], token));
         }