Closed Bug 1365018 Opened 2 years ago Closed 2 years ago

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

Categories

(DevTools :: Object Inspector, defect)

defect
Not set

Tracking

(firefox55 verified)

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attachment #8867904 - Flags: review?(dolske) → review?(thebnich+bmo)
Attachment #8867904 - Flags: review?(thebnich+bmo) → review?(MattN+bmo)
Attachment #8867904 - Flags: review?(MattN+bmo) → review?(nfitzgerald)
Attachment #8867904 - Flags: review?(nfitzgerald) → review?(ttromey)
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 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?
Attachment #8867904 - Flags: review?(ttromey)
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 ...
Assignee: nobody → oriol-bugzilla
Attachment #8867904 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8875101 - Flags: review?(ttromey)
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.
Attachment #8875101 - Flags: review?(ttromey) → review+
No, I can't push to try
Keywords: checkin-needed
Thanks for adding that.
I wonder if object inspector has this problem as well.
All things that use VariablesView have this problem.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a067d03f1627
Escape U+2028 and U+2029 when quoting a string in VariablesView. r=tromey
Keywords: checkin-needed
(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.
https://hg.mozilla.org/mozilla-central/rev/a067d03f1627
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
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.
Summary: Can't rename properties with U+2028 or U+2029 → Can't change the value of a property with U+2028 or U+2029
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
QA Whiteboard: [bugday-20170614]
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]
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]
As per Comment 14 & Comment 16, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.