Bug 211163 – Web Inspector: Uncaught Exception: SyntaxError: Unexpected identifier 'E'. Expected either a closing ']' or a ',' following an array element.

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

[Devin Rousso]

Uncaught Exception in Web Inspector.

# STEPS TO REPRODUCE
1. inspect any page
2. highlight "E" and press ⌘E
3. focus Web Inspector

Uncaught Exceptions:
-----------------------
 - SyntaxError: Unexpected identifier 'E'. Expected either a closing ']' or a ',' following an array element. (at Main.html:1:0)
-----------------------

Notes:
Inspected URL:        (unknown)
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/605.1.15 (KHTML, like Gecko)

[Devin Rousso]

(In reply to Devin Rousso from comment #0)
> 2. highlight "E" and press ⌘E
I meant the entire string `"E"`, not just the character `E` 😅

[Devin Rousso]

Created attachment 397932 [details]
Patch

[Devin Rousso]

Created attachment 397933 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 397933 [details]
Patch

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

> Source/WebKit/UIProcess/Inspector/mac/WebInspectorProxyMac.mm:205
> +        m_findString = findString;

Should this be cleared when the inspector closes so that we are forced to push to the frontend at least once when then frontend page is created?

[Devin Rousso]

Comment on attachment 397933 [details]
Patch

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

>> Source/WebKit/UIProcess/Inspector/mac/WebInspectorProxyMac.mm:205
>> +        m_findString = findString;
> 
> Should this be cleared when the inspector closes so that we are forced to push to the frontend at least once when then frontend page is created?

Actually, we probably don't want to do this at all, as it's possible for the find string to change in the frontend, but then be changed back to the previous value in another app:

1. select some text in any app and ⌘E
2. inspect any page
3. select some text in Web Inspector and ⌘E
4. repeat step #1
5. ⌘G in Web Inspector
 => the text from step #3 will be searched for

This is because `WebInspectorProxy` (and `RemoteWebInspectorProxy`) aren't told about changes to the find string that happen within Web Inspector, meaning that `m_findString` won't get updated, and therefore will cause the second ⌘E (step #4) to be interpreted as "oh we just saw the same string, don't tell the frontend" even though the find string in the frontend has changed.

[Devin Rousso]

Created attachment 397937 [details]
Patch

[EWS]

Committed r260887: <https://trac.webkit.org/changeset/260887>

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

[Radar WebKit Bug Importer]

<rdar://problem/62582698>

[Darin Adler]

Comment on attachment 397937 [details]
Patch

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

> Source/WTF/wtf/JSONValues.cpp:561
> +        // We could encode characters >= 127 as UTF-8 instead of \u escape sequences.
> +        // We could handle surrogates here if callers wanted that; for now we just
> +        // write them out as a \u sequence, so a surrogate pair appears as two of them.

If we wanted to handle surrogate pairs as a a single "\u" sequence, it would be so easy since there’s a StringView::codePoints that does half the work for us if we use it instead of StringView::codeUnits.

> Source/WTF/wtf/JSONValues.h:111
> +    static void escapeString(StringBuilder&, StringView);

The callers seem to want a String, not a StringBuilder. Why not expose it that way?