Bug 235061 – Web Inspector: `console.log` format strings containing invalid specifiers results in `[Object object]` replacing the specifier instead of ignoring the invalid specifier

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

# console.log print strings contains '%_' result is Object

## Reproduce code

console.log('__url__%__', 'url');

## Actual output

__url__[object Object]_

## Expected output

__url__%__ url

## Reference link

- https://console.spec.whatwg.org/#formatting-specifiers
- https://console.spec.whatwg.org/#formatter

[Radar WebKit Bug Importer]

<rdar://problem/87704591>

[Patrick Angle]

Created attachment 455289 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 455289 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=455289&action=review

r=me, interesting 🤔

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:1105
> +    value(format, substitutions, formatters, initialValue, append, {ignoreUnknownSpecifiers} = {})

I wonder is there a situation where we want to include an unknown specifier?  My gut tells me that this should be the default (and non-configurable) behavior.

> LayoutTests/inspector/unit-tests/string-utilities-expected.txt:23
> +WARN: String.format("%_ %s", "first", "second"): unsupported format character â_â. Treating as a string.

ewww we have smart quotes in error messages?  We should probably remove that (and switch to `WI.reportInternalError`).  Can be a followup if you'd prefer :)

[Patrick Angle]

Comment on attachment 455289 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=455289&action=review

>> Source/WebInspectorUI/UserInterface/Base/Utilities.js:1105
>> +    value(format, substitutions, formatters, initialValue, append, {ignoreUnknownSpecifiers} = {})
> 
> I wonder is there a situation where we want to include an unknown specifier?  My gut tells me that this should be the default (and non-configurable) behavior.

A quick survey of call sites seems to indicate that other usage either 1. is applied to localized strings, which should never contain an unknown specifier or 2. is acting on a string from the backend (IssueMessage) that should be a valid format string as well. ConsoleMessageView is kinda the exception in that we don't control the format string since it is author-provided.

>> LayoutTests/inspector/unit-tests/string-utilities-expected.txt:23
>> +WARN: String.format("%_ %s", "first", "second"): unsupported format character â_â. Treating as a string.
> 
> ewww we have smart quotes in error messages?  We should probably remove that (and switch to `WI.reportInternalError`).  Can be a followup if you'd prefer :)

If we make `ignoreUnknownSpecifiers` the default and non-configurable case, the first part of this will be irrelevant anyways. I'm not sure we want to reportInternalError, at least for the specific call-site of String.format we are dealing with here, since malformed user input shouldn't cause us to throw up the error handler in engineering builds. Every other (or almost every other?) call site to String.format probably should though... I'll do that as a followup so we can discuss that solution separately from this bug.

[Patrick Angle]

Created attachment 455392 [details]
Patch v1.1

[EWS]

Committed r291712 (248750@main): <https://commits.webkit.org/248750@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455392 [details].