Closed
Bug 988826
Opened 11 years ago
Closed 10 years ago
Use LongStringActor for exceptionMessage in the evaluateJS response packet
Categories
(DevTools :: Console, defect, P3)
DevTools
Console
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 36
People
(Reporter: msucan, Assigned: akshendra521994, Mentored)
Details
(Whiteboard: [lang=js])
Attachments
(1 file, 2 obsolete files)
|
1.72 KB,
patch
|
past
:
review+
|
Details | Diff | Splinter Review |
No description provided.
|
||
Updated•11 years ago
|
Mentor: mihai.sucan
Whiteboard: [mentor=msucan][lang=js] → [lang=js]
|
Assignee | |
Updated•10 years ago
|
Flags: needinfo?(mihai.sucan)
|
||
Comment 1•10 years ago
|
||
I'm not sure what the needinfo was for, but the purpose of this bug is to use a LongStringActor in the console backend for transmitting the exceptionMessage as it can get quite long. You just need to use _createStringGrip for this and you can crib from other uses of this function in the same file:
http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webconsole.js#788
Mentor: mihai.sucan → past
Flags: needinfo?(mihai.sucan)
|
|
Assignee | |
Comment 2•10 years ago
|
||
Attachment #8515499 -
Flags: review?(past)
|
|
||
Comment 3•10 years ago
|
||
Comment on attachment 8515499 [details] [diff] [review]
988826_LongStringActor.patch
Review of attachment 8515499 [details] [diff] [review]:
-----------------------------------------------------------------
Nice job! Now we need to write a test to make sure this won't regress in the future. Take a look at test_throw.html and see how it already verifies that the |exception| property is a long string.
http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/webconsole/test/test_throw.html#55
You only need to add a similar check for the exceptionMessage property.
Attachment #8515499 -
Flags: review?(past) → feedback+
|
|
Assignee | |
Comment 4•10 years ago
|
||
Please assign that to me!!
Attachment #8515499 -
Attachment is obsolete: true
Attachment #8523021 -
Flags: review?(past)
|
|
||
Comment 5•10 years ago
|
||
Comment on attachment 8523021 [details] [diff] [review]
988826_LongStringActor.patch
Review of attachment 8523021 [details] [diff] [review]:
-----------------------------------------------------------------
Nice, but it can be simplified even more.
::: toolkit/devtools/webconsole/test/test_throw.html
@@ +61,5 @@
>
> + tests.push(function() {
> + aState.client.evaluateJS("throw '" + longString + "';", function(aResponse) {
> + is(aResponse.exceptionMessage.initial, shortedString,
> + "exceptionMessage.initial for throw longString"
You don't need a new evaluateJS request, just to do the extra check in the previous one.
Attachment #8523021 -
Flags: review?(past) → feedback+
|
|
||
Updated•10 years ago
|
Assignee: nobody → akshendra521994
Status: NEW → ASSIGNED
|
|
Assignee | |
Comment 6•10 years ago
|
||
Attachment #8523021 -
Attachment is obsolete: true
Attachment #8523090 -
Flags: review?(past)
|
|
||
Comment 7•10 years ago
|
||
Comment on attachment 8523090 [details] [diff] [review]
988826_LongStringActor.patch
Review of attachment 8523090 [details] [diff] [review]:
-----------------------------------------------------------------
Perfect!
Attachment #8523090 -
Flags: review?(past) → review+
|
|
||
Comment 8•10 years ago
|
||
|
|
||
Updated•10 years ago
|
Keywords: checkin-needed
|
||
Comment 9•10 years ago
|
||
Keywords: checkin-needed
Whiteboard: [lang=js] → [lang=js][fixed-in-fx-team]
|
|
||
Comment 10•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [lang=js][fixed-in-fx-team] → [lang=js]
Target Milestone: --- → Firefox 36
|
||
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•