Closed Bug 833411 Opened 7 years ago Closed 7 years ago

Variables View: support for displaying long string actors

Categories

(DevTools :: Debugger, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 21

People

(Reporter: msucan, Assigned: msucan)

References

Details

Attachments

(1 file, 1 obsolete file)

It seems there's no specific support for displaying long string actors.
Attached patch fix (obsolete) — Splinter Review
This patch adds the .locked property for scopes and it also adds support for displaying long string actors.
Attachment #704950 - Flags: review?(vporof)
Comment on attachment 704950 [details] [diff] [review]
fix

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

::: browser/devtools/shared/VariablesView.jsm
@@ +2001,5 @@
>    if (!grip || typeof grip != "object") {
>      return true;
>    }
>  
> +  // For convenience, undefined, null nd long strings are considered primitives.

Typo: and.

@@ +2099,5 @@
>          return "undefined";
>        case "null":
>          return "null";
> +      case "longString":
> +        return "\"" + aGrip.initial + "\"";

Should we be adding an ellipsis here? Just wondering.
Attachment #704950 - Flags: review?(vporof) → review+
Attached patch typo fixSplinter Review
Thanks for the r+!
Attachment #704950 - Attachment is obsolete: true
(In reply to Victor Porof [:vp] from comment #2)
> Comment on attachment 704950 [details] [diff] [review]
> fix
> 
> Review of attachment 704950 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/shared/VariablesView.jsm
> @@ +2001,5 @@
> >    if (!grip || typeof grip != "object") {
> >      return true;
> >    }
> >  
> > +  // For convenience, undefined, null nd long strings are considered primitives.
> 
> Typo: and.

Yep, saw this after I submitted the patch.


> @@ +2099,5 @@
> >          return "undefined";
> >        case "null":
> >          return "null";
> > +      case "longString":
> > +        return "\"" + aGrip.initial + "\"";
> 
> Should we be adding an ellipsis here? Just wondering.

I also thought this should be the case. I learned when I did the LSA work for the web console that, in practice, the method that gives you the string for the given actor grip should not include the ellipsis. You need to have special-casing in the code to add the ellipsis, to link it, to remove it, and so on.
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [land-in-fx-team]
Whiteboard: [land-in-fx-team] → [f
https://hg.mozilla.org/mozilla-central/rev/db65cad1b6a4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 21
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.