Closed Bug 1310630 Opened 3 years ago Closed 2 years ago

Console should allow to get the full text of a longString

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(firefox49 unaffected, firefox50 unaffected, firefox51 unaffected, firefox52 disabled, firefox-esr52 disabled, firefox-esr60 wontfix, firefox53 disabled, firefox54 disabled, firefox55 disabled, firefox56 disabled, firefox57 wontfix, firefox59 wontfix, firefox60 wontfix, firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- disabled
firefox-esr52 --- disabled
firefox-esr60 --- wontfix
firefox53 --- disabled
firefox54 --- disabled
firefox55 --- disabled
firefox56 --- disabled
firefox57 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: marco, Assigned: nchevobbe)

References

Details

(Keywords: regression, Whiteboard: [newconsole-mvp])

Attachments

(1 file, 1 obsolete file)

fetch('http://ftp.mozilla.org/pub/firefox/nightly/2016/10/2016-10-01-03-04-30-mozilla-central/')
.then(response => response.text())
.then(data => {
  console.log(data);
})

This snippet used to work (data was not empty) before bug 1304178, doesn't work anymore now (data is empty).

Setting `devtools.webconsole.new-frontend-enabled` to false fixes the issue.

 5:43.79 INFO: Last good revision: 37f78aca862224d7151c0fcae1ed8373fe11c83b
 5:43.79 INFO: First bad revision: a5510966f80b9b2f5abf59ab32cf4c92d66c60de
 5:43.79 INFO: Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=37f78aca862224d7151c0fcae1ed8373fe11c83b&tochange=a5510966f80b9b2f5abf59ab32cf4c92d66c60de

 5:44.88 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1304178
The results are actually accessible from JS, so it's just console.log which is failing.
Component: DOM → Developer Tools: Console
Product: Core → Firefox
Summary: fetch returning an empty body → console.log printing an empty body after fetch
Thanks for filing. The behavior of the old console matches Chrome as well, so we should fix this before turning it on in Dev Edition.
Assignee: nobody → chevobbe.nicolas
This is due to LongString object not being handled by Reps, and will be fixed when Bug 1302982 lands
Depends on: 1302982
Priority: -- → P2
Comment on attachment 8804383 [details]
Bug 1310630 - Add Rep for LongString. ;

https://reviewboard.mozilla.org/r/88384/#review88448

Please see my inline comment.

Honza

::: devtools/client/shared/components/reps/long-string.js:51
(Diff revision 2)
> +        config.style = style;
> +      }
> +
> +      let croppedString = cropLimit && !(member && member.open)
> +        ? initial.substring(0, cropLimit)
> +        : initial;

Shouldn't we process the string by cropString to make sure e.g. any non-pritable characters are replaced?

Also, if the string is resolved and `_fullText` field available on the grip, shouldn't we use it?

This is the place where `_fullText` is set
https://dxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/client.js#631
Attachment #8804383 - Flags: review?(odvarko)
Something we must have in mind too : in the old console, clicking on the "[...]" part at the end of the truncated string show the full content of the long string, so we should have the same behavior in the new one.
BTW, I mixed the bug, I wanted to attach this patch for Bug 1302982 , I might push the next patch there, and handle the "retrieve the full content when clicking on the ellipsis" here.
> Also, if the string is resolved and `_fullText` field available on the grip, shouldn't we use it?

From what I understand, _fullText is not directly in the grip object (and I guess it would be hard to have since it's a promise), but could be passed as a property to the Rep.
Now, I wonder if we couldn't have the fullText in the `member` object, or maybe replace it.
The member object seems to be set in the DOM Panel when clicking on a truncated string, so maybe it'd make sense to retrieve the full text at that moment.
Attachment #8804383 - Attachment is obsolete: true
Attachment #8807229 - Flags: review?(odvarko)
Attachment #8807229 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 8807229 [details]
Bug 1310630 - Pass a createLongStringClient prop to the ObjectInspector; .

https://reviewboard.mozilla.org/r/90454/#review91554

Looks good, R+ assuming Try is green.
Attachment #8807229 - Flags: review+
Re-purposing this as Bug 1302982 landed and thus the console isn't broken anymore when logging a longString
Summary: console.log printing an empty body after fetch → Console should allow to get the full text of a longString
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #12)
> Re-purposing this as Bug 1302982 landed and thus the console isn't broken
> anymore when logging a longString

Nick, are the status flags still valid on this bug? It's showing up in regression triage.
Flags: needinfo?(chevobbe.nicolas)
I don't really know how regression flag works, but it can be seen as a regression since it works in the old console but not in the new one. 
The only thing that could make this not a regression is because the new console is preffed on only in Nightly, and this bug is a blocker for the bug where we plan to activate it everywhere (Bug 1308219).

Feel free to remove the flag if it makes sense to you.
Flags: needinfo?(chevobbe.nicolas)
We should still track this as a regression for whatever release eventually ships the new debugger. That said, setting the status for 52 to disabled since this is still Nightly-only.
No longer blocks: enable-new-console
In Bug 1358937 we will be able to display exceptionMessage sent as a longString in the console. This text will only be an excerpt of the full message since we'll only display the `initial` property and an ellipsis. We should also provide a way to "expand" the exception message to have its full content.
According to Bug 1397425, new frontend is riding with 57. Nicolas, are you still planning on working on this?
Flags: needinfo?(nchevobbe)
I am, let's add it to our tracking list
Flags: needinfo?(nchevobbe)
Whiteboard: [reserve-console-html]
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify?
Priority: P2 → P1
Iteration: 57.3 - Sep 19 → ---
See Also: → 968976
Blocks: 1403448
Priority: P1 → P3
Assignee: nchevobbe → nobody
Status: ASSIGNED → NEW
Blocks: 1406192
Priority: P3 → P2
Whiteboard: [reserve-console-html] → [newconsole-mvp]
Flags: qe-verify?
Duplicate of this bug: 1418382
Duplicate of this bug: 1420257
Blocks: 1336097
Depends on: 1452566
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment on attachment 8807229 [details]
Bug 1310630 - Pass a createLongStringClient prop to the ObjectInspector; .

https://reviewboard.mozilla.org/r/90454/#review244284
Attachment #8807229 - Flags: review?(bgrinstead) → review+
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b0d116ec4546
Pass a createLongStringClient prop to the ObjectInspector; r=bgrins,Honza.
https://hg.mozilla.org/mozilla-central/rev/b0d116ec4546
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.