1365018 - Can't change the value of a property with U+2028 or U+2029

Status: VERIFIED FIXED

(DevTools :: Object Inspector, defect)

Description

[Oriol Brufau [:Oriol]]

In bug 996691 I fixed some injection problems (see details in bug 1296981).

But I made a mistake: not all JSON strings are valid JS strings. Specifically, they can contain U+2028 or U+2029, which are considered line terminators in ECMAScript.

1. Open the console
2. Enter `inspect({"\u2028": 1})`
3. In the object inspector, click `1`
4. Write `2` and press enter

The value does not change.

Comment 1

[Oriol Brufau [:Oriol]]

Attachment: Bug 1365018 - Escape U+2028 and U+2029 when quoting a string in VariablesView.

Review commit: https://reviewboard.mozilla.org/r/139430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139430/

Comment 2

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

Hey Oriol, I'm not hacking on the devtools anymore, but tromey is a good person to send patches related to the variables view. Cheers!

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8867904 [details]
Bug 1365018 - Escape U+2028 and U+2029 when quoting a string in VariablesView.

https://reviewboard.mozilla.org/r/139430/#review146634

Thank you for the patch.  This looks reasonable to me, but is it possible to write a regression test?

Comment 4

[Oriol Brufau [:Oriol]]

Attachment: variablesview-escape-v2.patch

Finally it worked! Debugger tests are tricky as hell.

It would have been easier if I (or you, Nick, cheers!) had noticed this in bug 996691 ...

Comment 5

[Tom Tromey :tromey]

Comment on attachment 8875101 [details] [diff] [review]
variablesview-escape-v2.patch

Review of attachment 8875101 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for doing this.  This looks good to me.
Can you push it through try (I don't know); if not, let me know and I will.

Comment 6

[Oriol Brufau [:Oriol]]

No, I can't push to try

Comment 7

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5abc6c3fc6170d8e73fcd8386be0aa08854fb508

Comment 8

[Tom Tromey :tromey]

Thanks for adding that.
I wonder if object inspector has this problem as well.

Comment 9

[Oriol Brufau [:Oriol]]

All things that use VariablesView have this problem.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a067d03f1627
Escape U+2028 and U+2029 when quoting a string in VariablesView. r=tromey

Comment 11

[Tom Tromey :tromey]

(In reply to Oriol [:Oriol] from comment #9)
> All things that use VariablesView have this problem.

Sorry if I wasn't clear.  Object inspector is the replacement for VariablesView that is in the new debugger,
and will soon be in the new console.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a067d03f1627

Comment 13

[Oriol Brufau [:Oriol]]

The title of the bug was wrong, what this fixes is changing property values.

Renaming still fails with U+2028, U+2029, quotes, and probably other things. This should be addressed in bug 1296981.

Comment 14

[Sajedul Islam]

I have reproduced this bug with Nightly 55.0a1 (2017-05-15) on Windows 10 , 64 Bit ! 

This bug's fix is Verified with latest Nightly 56.0a1 !

Build   ID    20170613030203
User Agent    Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0

Comment 15

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 55.0a1 (2017-05-15) on Ubuntu 16.04 LTS!

This bug's fix is Verified with latest Nightly !

Build   ID : 20170613030203

User Agent : Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170614]

Comment 16

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 55.0a1 (2017-05-15) (64-bit) on Ubuntu 16.04 LTS!

This bug's fix is Verified with latest Nightly !

Build   ID : 20170613100218
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

[bugday-20170614]

Comment 17

[Sajedul Islam]

As per Comment 14 & Comment 16, I am marking this bug as verified fixed.