Closed Bug 1310801 Opened 8 years ago Closed 8 years ago

web console stops printing output after attempting to print a string with non-printable characters

Categories

(DevTools :: Shared Components, defect, P2)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: keeler, Assigned: Honza)

References

Details

Attachments

(1 file, 3 obsolete files)

STR:

Open the web console
Type (including quotes) '\x00'
Hit enter
Type anything else (and hit enter again)

Expected results:

The console should probably display something for the '\x00' string, but that's open to interpretation. It should definitely keep displaying other things, though.

Actual results:

The console just goes silent, and won't display anything else until you close/reopen it.
Component: Developer Tools → Developer Tools: Console
This is likely an issue with Reps, which both the console and debugger use to output values received from the backend. Honza, do you have any thoughts? Feel free to bounce it back to console if I'm wrong about that.
Component: Developer Tools: Console → Developer Tools: Shared Components
Flags: needinfo?(odvarko)
Attached patch bug1310801.patch (obsolete) — Splinter Review
The problem is in StringRep. The attached patch removes non-printable characters before rendering in HTML using regular expression. But, keeping \n and \t.

This certainly fixes the problem, but I am not 100% sure if the reg exp is comprehensive enough. We might also want to use something as follows:

http://stackoverflow.com/questions/11598786/how-to-replace-non-printable-unicode-characters-javascript
(see the first answer)

Or am I missing something? Is there a simple solution?

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Flags: needinfo?(odvarko)
Attachment #8802536 - Flags: review?(chevobbe.nicolas)
Comment on attachment 8802536 [details] [diff] [review]
bug1310801.patch

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

The regex is not that easy to understand, but since you added a comment, that's fine.
I wonder if we could do this replace in the `cropString` function. 
Since it is used for the comment rep and the text node one, it would handle this issue for those, which would be nice.
Also, this need some tests to ensure the problem is fixed.

::: devtools/client/shared/components/reps/string.js
@@ +50,5 @@
>  
> +      // Remove all non-printable characters.
> +      // eslint-disable-next-line no-control-regex
> +      let re = new RegExp("[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\x9F]", "g");
> +      formattedString = formattedString.replace(re, "");

In the current console, we display a "" character for those, maybe we still want to have it ?
Attachment #8802536 - Flags: review?(chevobbe.nicolas) → review-
Attached patch bug1310801.patch (obsolete) — Splinter Review
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #3)
> Comment on attachment 8802536 [details] [diff] [review]
> bug1310801.patch
> 
> Review of attachment 8802536 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The regex is not that easy to understand, but since you added a comment,
> that's fine.
I yet improved the comment a bit.

> I wonder if we could do this replace in the `cropString` function. 
> Since it is used for the comment rep and the text node one, it would handle
> this issue for those, which would be nice.
Good point, done.

> Also, this need some tests to ensure the problem is fixed.
I extended the test for StringRep.

> 
> ::: devtools/client/shared/components/reps/string.js
> @@ +50,5 @@
> >  
> > +      // Remove all non-printable characters.
> > +      // eslint-disable-next-line no-control-regex
> > +      let re = new RegExp("[\x00-\x08\x0B\x0C\x0E-\x1F\x80-\x9F]", "g");
> > +      formattedString = formattedString.replace(re, "");
> 
> In the current console, we display a "" character for those, maybe we still
> want to have it ?
The characters has been mangled, but I agree we can display something instead of it . Do you have any specific char in mind. What's the code?

Honza
Attachment #8802536 - Attachment is obsolete: true
Attachment #8802992 - Flags: review?(chevobbe.nicolas)
> The characters has been mangled, but I agree we can display something instead of it . Do you have any specific char in mind. What's the code?

Seems that I can't copy it. If you can, enter "\x00" in the old console frontend (can be the jsconsole since it doesn't use the new frontend yet).
Comment on attachment 8802992 [details] [diff] [review]
bug1310801.patch

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

Seems good to me !

::: devtools/client/shared/components/reps/rep-utils.js
@@ +47,5 @@
>  
>      // Make sure it's a string.
>      text = text + "";
>  
> +    // Remove all non-printable characters except of

Nice indeed
Attachment #8802992 - Flags: review?(chevobbe.nicolas) → review+
Attached patch bug1310801.patch (obsolete) — Splinter Review
Thank for the review!

Patch updated: unicode replacement characters u+fffd used to replace non-printable characters.

Honza
Attachment #8802992 - Attachment is obsolete: true
Attachment #8803302 - Flags: review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb432564f4d8
Remove non-printable characters. r=nchevobbe
Keywords: checkin-needed
Priority: -- → P2
Attached patch bug1310801.patchSplinter Review
Test fixed

Honza
Attachment #8803302 - Attachment is obsolete: true
Attachment #8805082 - Flags: review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/68f683a2034b
Remove non-printable characters. r=nchevobbe
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/68f683a2034b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
I have reproduced this bug with Nightly 52.0a1 (2016-10-17) (64-bit) on Windows 7 , 64 bit !

This bug's fix is verified with latest Nightly
 
Build ID        20161102030205
User Agent      Mozilla/5.0 (WindowsNT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0  
[bugday-20161102]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.