1258584 - Newlines don't show up in variables view sidebar

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Screen Shot 2016-03-21 at 5.43.35 PM.png

Yikes...

https://twitter.com/jonathansampson/status/711988345358802948

STR:

Open this in the debugger:

"""
<script>
  var s = "\nfoo\nbar\nbaz\n";
  debugger;
</script>
"""

Comment 1

[James Long (:jlongster)]

So here's an interesting question: how should we output symbols with a newline? You can do `Symbol("foo\nbar")`. Should we run that name through the same process that we do strings? Seems like we should, but right now symbols are shown as `Symbol(foo)`, meaning it's not obvious that we're stringifying the name. Does `Symbol(foo\nbar)` have a name of "foo\\nbar" or "foo\nbar"? We could solve this by adding quotes around the name.

Comment 2

[James Long (:jlongster)]

Attachment: 1258584.patch

This patch calls `JSON.stringify` on strings. This takes care of all values that need to be escaped. There are all kind of other characters that aren't showing up: `\r` \t` etc. Basically, we are just putting the raw string straight into the DOM.

Need to solve `Symbol` as well, see comment above. Will fix/add tests if you think this approach is right.

Comment 3

[Tom Tromey :tromey]

I spent an unusual amount of time rediscovering this bug yesterday.
It happens with the console as well.  STR:

x = {a:"string with\nnewline", b:73, c:92}
then inspect the resulting object.

The variable display shows

a: "string withnewline"

I think it should show:

a: "string with\nnewline"

Comment 4

[Tom Tromey :tromey]

It's worth considering many cases when displaying strings.
Enumerating the ones I know of:

* control characters - https://en.wikipedia.org/wiki/Unicode_control_characters
  I think it would be fine to use \uNNNN for these, Unicode provides
  "control character pictures", but I had never heard of these until reading
  that article, so it's not clear to me that displaying these would help much.
  Perhaps the bidi text control characters could be left alone.

* zero-width characters - I know of ZWJ, ZWNJ, and ZWSP.
  It may make sense to display these specially as \uNNNN.

* A BOM should probably be displayed explicitly.

* Unpair surrogate characters should be escaped.

For other unusual cases, say zalgo text, I think it's fine to just display it.
The main principle is to make sure that what is there is viewable somehow -- so
otherwise-invisible characters ought to be made visible.  (Perhaps this argues for
escaping bidi controls, I'm not sure what is best).

Comment 6

[Tom Tromey :tromey]

Let's not forget that backslash and double quote have to be escaped.

Also maybe it would be nice to have some kind of "show as ascii" button for strings
that aren't entirely ascii already.  This would quote everything outside [32,127].

Comment 7

[Tom Tromey :tromey]

Fixed in devtools-reps.

Comment 8

[Tom Tromey :tromey]

... except not truly fixed, because it also affects the variables view, which I didn't touch.

Comment 10

[Tom Tromey :tromey]

Fixing VariablesView is relatively straightforward:

* Export escapeString from devtools-reps
* Use that in VariablesView.jsm (see VariablesView.stringifiers.byType) to quote the strings

However this isn't worth doing if the object inspector change is happening imminently.

Comment 11

[Tom Tromey :tromey]

This is fixed in nightly when using the new debugger.