1621551 - DevTools collapses spaces in debugger value display

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Glenn Linderman]

Attachment: inputbug.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:73.0) Gecko/20100101 Firefox/73.0

Steps to reproduce:

open attached file inputbug.html in browser

Set an event breakpoint on the change event.

type some non-blank characters, followed by several spaces, followed by some non-blank characters, finally with a Tab or Enter to trigger the change.

Go to the

Actual results:

Change event happened. Breakpoint hit. Expand event.target.
Observe the value shows only one space when there should be several as typed.
Go from the breakpoint, and the console log confirms there are several spaces, as entered.

Expected results:

Viewing string values in the debugger (this happens not only with .value, but any string variable containing multiple spaces) should not collapse spaces. It is confusing to the user. At first I thought maybe the <input type=text> was specified to eliminate spaces, maybe, went and read the spec, only CR LF are suppressed. Then I wondered if somehow javascript string replace was updating the value of the original string, as well and the return string, because my real code was trying to eliminate those spaces, but according to the debugger, they weren't arriving! I eventually wrote this test case, and tried it in Chrome. Chrome DevTools shows the spaces in the value, and I finally figured out that it was the Firefox DevTools lying about the value.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jlast, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

I can easily reproduce this issue

STR:

1. Load the https://bug1621551.bmoattachments.org/attachment.cgi?id=9132495
2. Create Event BP Control -> Change
3. Type abc abc (more than once space at the beggining and end and some in the middle)
4. Click on the page (to move focus out of the input) -> hit the BP
5. Type event.target.value in the Watch Expressions panel
6. Check out the value, the spaces are collapsed to one: abc abc => BUG

ER: amount of spaces should correspond to the actual value.

Honza

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Jason, could this be good first bug?

Honza

Comment 4

[Aaryan Dewan]

Hi. I am able to reproduce it. Can I work on it as my first bug?

Also, could you please give me some more information as in how should one tackle it?
I searched the code base and I think it might be caused by the L10N.getStr method.

Comment 5

[Glenn Linderman]

(In reply to Aaryan Dewan from comment #4)

Hi. I am able to reproduce it. Can I work on it as my first bug?

Also, could you please give me some more information as in how should one tackle it?
I searched the code base and I think it might be caused by the L10N.getStr method.

Hi Aaryan, I'm not sure of all the protocol here, but I think needinfo is an alert for the original bug reporter (for this bug, that is me), rather than for folks that actually work on the Firefox code base. Anyway, I got your alert, and I sort of doubt anyone else did. I see in the bottom of the screen now that one can redirect needinfo requests, so I tried redirecting to the triage owner, but I'm not sure if that is the right protocol either. I'm just a user of Firefox, not a developer of it.

Comment 6

[Aaryan Dewan]

That's fine Glenn. Thank you for reporting the bug :)

Comment 7

[:Harald Kirschner :digitarald]

Thanks for jumping in Aaryan (and Glenn for being responsive to ni?, much appreciated!)

David, could you mentor this one?

Comment 8

[David Walsh :davidwalsh]

Sure, I'd love to. Welcome Aaryan! Please join our Slack channel (https://devtools-html-slack.herokuapp.com/) #debugger to get in touch!

This is an interesting problem for sure. Both the preview tooltip inside the debugger editor and the watch expressions panel strip spaces, while going to the console and typing event.target.value keeps the spaces. Typing event.target in the console, and then navigating down to value also keeps the spaces, so this seems debugger-specific. Feel free to start looking in the devtools source tree. I'll be looking at this later today to get more concrete ideas where this problem could be originating.

Comment 9

[David Walsh :davidwalsh]

Looked at this a bit more quickly; I made this update to see the string values we receive:

https://gist.github.com/darkwing/2bf673f392e5f517ca3721cf0c05b6b5

We are receiving the values correctly in the Watch Expressions render, so I think ObjectInspector is modifying the values. Have a look through the devtools/client/debugger/packages/devtools-reps/ files. I'd recommend doing a search for replace( or \w to find possible spots where we're modifying that values.

Comment 10

[David Walsh :davidwalsh]

Attachment: BrowserSpacing.png

OK, so I looked further into this and it's not a debugger or reps issue; it's a browser rendering thing. When I look at the raw HTML output in both the Expression output, it does contain all of the correct spaces, as shown in this screenshot.

We may need to, unfortunately, look at using dangerouslySetInnerHTML. Let me know if you have any questions!

Comment 11

[Glenn Linderman]

Not being familiar with DevTools internals, but realizing that blank space collapsing was a standard browser feature, and that DevTools is probably using browser rendering to do its job, I assumed that the problem was "simply" that the CSS for this display of string data needed to have "white-space: pre" included??? I don't know anything about dangerouslySetInnerHTML. Of course, I realize there are issue about long strings getting shortened for display with ... also. Ignore this comment if it is not relevant.

Comment 12

[David Walsh :davidwalsh]

Aaryan: After a wild search I think it boils down to a one line fix:

diff --git a/devtools/client/debugger/src/components/SecondaryPanes/Scopes.css b/devtools/client/debugger/src/components/SecondaryPanes/Scopes.css
--- a/devtools/client/debugger/src/components/SecondaryPanes/Scopes.css
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Scopes.css
@@ -46,7 +46,6 @@ html[dir="rtl"] .object-node {
 }
 
 .objectBox-object,
-.objectBox-string,
 .objectBox-text,
 .objectBox-table,
 .objectLink-textNode,

Please test out and let me know if it works for you. It should fix this problem in the Scopes, Expressions, and Preview components. Let's talk on Slack about next steps :)

Comment 13

[David Walsh :davidwalsh]

@Glenn: Heh, we came to the same conclusion at the same time. Well done :)

Comment 14

[Aaryan Dewan]

Attachment: Bug 1621551 - Remove objectBox-string selector that was responsible for white-space: nowrap. r=davidwalsh

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b43e52af036
Remove objectBox-string selector that was responsible for white-space: nowrap. r=davidwalsh

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2b43e52af036