Closed
Bug 988826
Opened 10 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•10 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
|
||
https://tbpl.mozilla.org/?tree=Try&rev=0ce00da17722
Updated•10 years ago
|
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/ad98a5bdb2e1
Keywords: checkin-needed
Whiteboard: [lang=js] → [lang=js][fixed-in-fx-team]
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ad98a5bdb2e1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [lang=js][fixed-in-fx-team] → [lang=js]
Target Milestone: --- → Firefox 36
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•