Last Comment Bug 755346 - Global variables are not displayed in the debugger frontend
: Global variables are not displayed in the debugger frontend
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: Firefox 15
Assigned To: Panos Astithas [:past]
:
Mentors:
: 753294 755787 (view as bug list)
Depends on:
Blocks: minotaur 755787 757282 758157 759045
  Show dependency treegraph
 
Reported: 2012-05-15 09:39 PDT by Panos Astithas [:past]
Modified: 2012-06-02 06:53 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.49 KB, patch)
2012-05-15 09:43 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v2 (12.26 KB, patch)
2012-05-16 08:31 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v3 (16.85 KB, patch)
2012-05-16 12:56 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v4 (24.22 KB, patch)
2012-05-17 02:59 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v5 (27.79 KB, patch)
2012-05-17 06:25 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v6 (44.37 KB, patch)
2012-05-17 12:06 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v7 (53.01 KB, patch)
2012-05-18 06:44 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
v6-v7 diff (17.92 KB, patch)
2012-05-18 06:46 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v8 (56.42 KB, patch)
2012-05-21 06:52 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Patch v9 (59.63 KB, patch)
2012-05-25 01:45 PDT, Panos Astithas [:past]
rcampbell: review+
Details | Diff | Review
v9, part2 (6.10 KB, patch)
2012-05-25 11:43 PDT, Victor Porof [:vporof][:vp]
past: review+
Details | Diff | Review
Patch v10 (59.52 KB, patch)
2012-05-28 12:27 PDT, Panos Astithas [:past]
no flags Details | Diff | Review
Fix for test breakage (879 bytes, patch)
2012-05-28 13:44 PDT, Panos Astithas [:past]
vporof: review+
Details | Diff | Review

Description Panos Astithas [:past] 2012-05-15 09:39:30 PDT
Even though the Debugger API supports it, we don't seem to show the global variables in the UI.
Comment 1 Panos Astithas [:past] 2012-05-15 09:43:57 PDT
Created attachment 624079 [details] [diff] [review]
Patch v1

We were missing the code to populate the global variables pane, and a couple of fixes to the handling of parent environments, mandated by the recent refactoring. This patch needs tests and to figure out what to display where, when the current frame is in the global environment.
Comment 2 Panos Astithas [:past] 2012-05-16 08:31:23 PDT
Created attachment 624390 [details] [diff] [review]
Patch v2

This is now ready for review.
Comment 3 Panos Astithas [:past] 2012-05-16 09:42:51 PDT
Comment on attachment 624390 [details] [diff] [review]
Patch v2

Actually, scratch that. I now realize that I had the protocol spec for global environments wrong, even though the user experience was correct. I'll fix that and I'll throw in the fix for |with| scope as a bonus.
Comment 4 Panos Astithas [:past] 2012-05-16 09:44:07 PDT
*** Bug 755787 has been marked as a duplicate of this bug. ***
Comment 5 Panos Astithas [:past] 2012-05-16 12:56:43 PDT
Created attachment 624501 [details] [diff] [review]
Patch v3

Working global scope and |with| scope, fixed protocol implementation. I still need to add tests for the |with| scope and make sure corner cases are handled correctly (e.g. |with| in global scope, or |with| inside |with|).
Comment 6 Panos Astithas [:past] 2012-05-17 02:59:57 PDT
Created attachment 624685 [details] [diff] [review]
Patch v4

Added many more tests.
Comment 7 Panos Astithas [:past] 2012-05-17 06:25:25 PDT
Created attachment 624721 [details] [diff] [review]
Patch v5

Some minor actor changes after bug 755778.
Comment 8 Panos Astithas [:past] 2012-05-17 12:06:30 PDT
Created attachment 624825 [details] [diff] [review]
Patch v6

After changing my mind a couple of times already, I'm now certain that this is what we should be doing in the variables view. There is no point to having a static set of scopes and populating them. This may be better than what Firebug does, but is far less than what WebKit does. Frames can have any number of nested scopes and we must present them all, if we are to do a meaningful job. This expands the scope of the original patch, but I can't believe we were restricting the display of scope information, essentially throwing away a lot of what the debugger server was sending our way. This was all my fault, alas.

I also made sure that the environment bindings is now in accordance to the spec for all types of environments and added new tests. This display now allows us to do more advanced stuff, like detect overriden variables and display them with a strike-through font, like the rule view. Victor, you should take a thorough look into my DebuggerView changes, to see if I missed anything.
Comment 9 Panos Astithas [:past] 2012-05-17 12:47:14 PDT
*** Bug 753294 has been marked as a duplicate of this bug. ***
Comment 10 Panos Astithas [:past] 2012-05-18 06:44:41 PDT
Created attachment 625086 [details] [diff] [review]
Patch v7

Moar tests! Coincidentally, these uncovered some obscure bugs that I had to fix as well. I promise, this is the last version of the patch. Scout's honor.
Comment 11 Panos Astithas [:past] 2012-05-18 06:46:10 PDT
Created attachment 625088 [details] [diff] [review]
v6-v7 diff

If you were already half way through the previous patch, you can use this diff to see what has changed.
Comment 12 Panos Astithas [:past] 2012-05-21 06:52:16 PDT
Created attachment 625630 [details] [diff] [review]
Patch v8

Rebased on top of bug 752770 and simplified accordingly. If you are already midway through reviewing v7, just ignore this.
Comment 13 Rob Campbell [:rc] (:robcee) 2012-05-24 11:54:10 PDT
firstly, has this been through try yet?
Comment 14 Rob Campbell [:rc] (:robcee) 2012-05-24 11:56:52 PDT
secondly, I'm getting the weird startup and shutdown bug every time I try to launch the debugger with this patch.

This is on the Error Console:

Timestamp: 2012-05-24 15:56:17
Error: [Exception... "Component returned failure code: 0x80470002 (NS_BASE_STREAM_CLOSED) [nsIOutputStream.write]"  nsresult: "0x80470002 (NS_BASE_STREAM_CLOSED)"  location: "JS frame :: chrome://global/content/devtools/dbg-transport.js :: DT_onOutputStreamReady :: line 94"  data: no]
Source File: chrome://global/content/devtools/dbg-transport.js
Line: 94

I'll try activating it with debugdump enabled.
Comment 15 Rob Campbell [:rc] (:robcee) 2012-05-24 11:59:45 PDT
attempted with debug logging and it worked fine on first open. However, on shutdown, I received this:

2012-05-24 15:58:05.631 firefox[29177:507] *** Assertion failure in -[mozRootAccessible didReceiveFocus], /Volumes/Data/FXTEAM/fx-team-working/accessible/src/mac/mozAccessible.mm:584
2012-05-24 15:58:05.632 firefox[29177:507] Mozilla has caught an Obj-C exception [NSInternalInconsistencyException: trying to set focus to ignored element! ((0x150bb31c0) AXUnknown)]
DBG-SERVER: Cleaning up connection.
DBG-SERVER: Cleaning up connection.
DBG-SERVER: Cleaning up connection.

the three Cleaning up connection messages are from three separate attempts to launch the debugger.
Comment 16 Panos Astithas [:past] 2012-05-25 01:45:03 PDT
Created attachment 627133 [details] [diff] [review]
Patch v9

Sorry, the recent landings seriously bitrotted this patch. I've updated it and now everything works as expected. Unfortunately this patch regresses bug 724229 (brief variable flashing) and it doesn't look like it's a trivial amount of work for me to fix that. Victor, any help would be appreciated, but I think that a different strategy may be required, now that there are no static scopes between pauses.

Try run:
https://tbpl.mozilla.org/?tree=Try&rev=5e9c023ee947
Comment 17 Victor Porof [:vporof][:vp] 2012-05-25 04:00:14 PDT
(In reply to Panos Astithas [:past] from comment #16)
> Created attachment 627133 [details] [diff] [review]
> Patch v9
> 
> Sorry, the recent landings seriously bitrotted this patch. I've updated it
> and now everything works as expected. Unfortunately this patch regresses bug
> 724229 (brief variable flashing) and it doesn't look like it's a trivial
> amount of work for me to fix that. Victor, any help would be appreciated,
> but I think that a different strategy may be required, now that there are no
> static scopes between pauses.
> 
> Try run:
> https://tbpl.mozilla.org/?tree=Try&rev=5e9c023ee947

I'll take a look.
Comment 18 Victor Porof [:vporof][:vp] 2012-05-25 11:42:46 PDT
(In reply to Panos Astithas [:past] from comment #16)
> Created attachment 627133 [details] [diff] [review]
> 

+            objClient.getPrototypeAndProperties(function SF_getProps(aResponse) {
+              this._addScopeVariables(aResponse.ownProperties, scope);
+              // Signal that variables have been fetched.
+              DebuggerController.dispatchEvent("Debugger:FetchedVariables");
+            }.bind(this));

This makes me a bit worried because there's another signal at the end of the function. Therefore, we get multiple FetchedVariables events dispatched, but somebody has no idea what scope they refer to, or if they actually mean "I got all the variables from all the scopes, you can start testing now".

-   * Removes all elements from the stackframes container, and adds a child node
+   * Removes all elements from the variables container, and adds a child node

I don't think you want to change that there.
The comments are the other way around: stackframes for DVF_emptyText and variables for DVP_emptyText.

-  this._addScope = this._addScope.bind(this);
+  this.addScope = this._addScope.bind(this);
+  this.clearScopes = this._clearScopes.bind(this);

No need to bind clearScopes to _clearScopes. Just make it directly public in the prototype, since it's always called from an instance.
I'd also like it renamed to empty, to preserve the same interface with StackframesView, since most of their functions have similar naming.

+  /**
+   * Removes all elements from the stackframes container, and adds a child node
+   * with an empty text note attached.
+   */
+  emptyText: function DVP_emptyText() {

Same thing.
s/stackframes/properties

+    // Generate a unique id for the element, if not specified.
+    aId = aId || aName.toLowerCase().trim().replace(" ", "-") + this._idCount++;
 
Should now be
aId = aId || aName.toLowerCase().trim().replace(/\s+/g, "-") + this._idCount++;
since we can have more than one space and the naming can be more volatile.
I'll take care of this in the followup.

+        // Construct the scope name.
+        let name = env.type.substring(0, 1).toUpperCase() +
+                   env.type.substring(1);

How about
let name = env.type.charAt(0).toUpperCase() + env.type.slice(1);
Fits on one line :)

-    this._localScope = this._addScope(L10N.getStr("localScope")).expand();
-    this._withScope = this._addScope(L10N.getStr("withScope")).hide();
-    this._closureScope = this._addScope(L10N.getStr("closureScope")).hide();
-    this._globalScope = this._addScope(L10N.getStr("globalScope"));

Should've also removed
this._saveHierarchy({ element: this._localScope, store: this._currHierarchy });
and friends from DVP_createHierarchyStore, but I'll take care of that in the followup patch.


+++ b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js

This test does absolutely nothing now :) Maybe remove it and rename the others? Or add something in it, I don't know.
Comment 19 Victor Porof [:vporof][:vp] 2012-05-25 11:43:14 PDT
Created attachment 627307 [details] [diff] [review]
v9, part2
Comment 20 Panos Astithas [:past] 2012-05-27 00:06:26 PDT
Comment on attachment 627307 [details] [diff] [review]
v9, part2

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

Perfect, thanks! I had to resort myself to nitpicking.

::: browser/devtools/debugger/debugger-view.js
@@ +811,5 @@
>  
> +    /**
> +     * @see DebuggerView.Properties.addScopeToHierarchy
> +     */
> +    element.addToHierarchy = this.addScopeToHierarchy.bind(this, element);

I'm not used to seeing this type of comment not preceding function declarations.

@@ +1684,3 @@
>  
> +  /**
> +   * Creates a designated hierarchy holder for a scope.

s/designated// (too wordy?)
Comment 21 Panos Astithas [:past] 2012-05-27 00:28:20 PDT
(In reply to Victor Porof from comment #18)
> (In reply to Panos Astithas [:past] from comment #16)
> > Created attachment 627133 [details] [diff] [review]
> > 
> 
> +            objClient.getPrototypeAndProperties(function
> SF_getProps(aResponse) {
> +              this._addScopeVariables(aResponse.ownProperties, scope);
> +              // Signal that variables have been fetched.
> +              DebuggerController.dispatchEvent("Debugger:FetchedVariables");
> +            }.bind(this));
> 
> This makes me a bit worried because there's another signal at the end of the
> function. Therefore, we get multiple FetchedVariables events dispatched, but
> somebody has no idea what scope they refer to, or if they actually mean "I
> got all the variables from all the scopes, you can start testing now".

Since this event was for use by tests only, I opted for triggering one after every protocol response was received, and let the test make sure it got all it needed. I see now that you are counting on it in SF_onFetchedVars, which I hadn't anticipated. It seems to me that making sure FetchedVariables is fired only once can be done in a followup, without serious problems. What do you think?

> -   * Removes all elements from the stackframes container, and adds a child
> node
> +   * Removes all elements from the variables container, and adds a child
> node
> 
> I don't think you want to change that there.
> The comments are the other way around: stackframes for DVF_emptyText and
> variables for DVP_emptyText.

Oops!

> -  this._addScope = this._addScope.bind(this);
> +  this.addScope = this._addScope.bind(this);
> +  this.clearScopes = this._clearScopes.bind(this);
> 
> No need to bind clearScopes to _clearScopes. Just make it directly public in
> the prototype, since it's always called from an instance.
> I'd also like it renamed to empty, to preserve the same interface with
> StackframesView, since most of their functions have similar naming.

OK, done.

> +  /**
> +   * Removes all elements from the stackframes container, and adds a child
> node
> +   * with an empty text note attached.
> +   */
> +  emptyText: function DVP_emptyText() {
> 
> Same thing.
> s/stackframes/properties

Done.

> +    // Generate a unique id for the element, if not specified.
> +    aId = aId || aName.toLowerCase().trim().replace(" ", "-") +
> this._idCount++;
>  
> Should now be
> aId = aId || aName.toLowerCase().trim().replace(/\s+/g, "-") +
> this._idCount++;
> since we can have more than one space and the naming can be more volatile.
> I'll take care of this in the followup.

I don't think there can ever be more than one consecutive spaces, but OK.

> +        // Construct the scope name.
> +        let name = env.type.substring(0, 1).toUpperCase() +
> +                   env.type.substring(1);
> 
> How about
> let name = env.type.charAt(0).toUpperCase() + env.type.slice(1);
> Fits on one line :)

Tasty! Done.

> -    this._localScope = this._addScope(L10N.getStr("localScope")).expand();
> -    this._withScope = this._addScope(L10N.getStr("withScope")).hide();
> -    this._closureScope = this._addScope(L10N.getStr("closureScope")).hide();
> -    this._globalScope = this._addScope(L10N.getStr("globalScope"));
> 
> Should've also removed
> this._saveHierarchy({ element: this._localScope, store: this._currHierarchy
> });
> and friends from DVP_createHierarchyStore, but I'll take care of that in the
> followup patch.
> 
> 
> +++ b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js
> 
> This test does absolutely nothing now :) Maybe remove it and rename the
> others? Or add something in it, I don't know.

It still tests the shortening of script labels, so I figured it did something useful. I renamed the test function to testScriptLabelShortening, in order for that to be more evident.
Comment 22 Victor Porof [:vporof][:vp] 2012-05-27 10:44:23 PDT
(In reply to Panos Astithas [:past] from comment #21)
> (In reply to Victor Porof from comment #18)
> > (In reply to Panos Astithas [:past] from comment #16)
> > > Created attachment 627133 [details] [diff] [review]
> > > 
> > 
> > +            objClient.getPrototypeAndProperties(function
> > SF_getProps(aResponse) {
> > +              this._addScopeVariables(aResponse.ownProperties, scope);
> > +              // Signal that variables have been fetched.
> > +              DebuggerController.dispatchEvent("Debugger:FetchedVariables");
> > +            }.bind(this));
> > 
> > This makes me a bit worried because there's another signal at the end of the
> > function. Therefore, we get multiple FetchedVariables events dispatched, but
> > somebody has no idea what scope they refer to, or if they actually mean "I
> > got all the variables from all the scopes, you can start testing now".
> 
> Since this event was for use by tests only, I opted for triggering one after
> every protocol response was received, and let the test make sure it got all
> it needed. I see now that you are counting on it in SF_onFetchedVars, which
> I hadn't anticipated. It seems to me that making sure FetchedVariables is
> fired only once can be done in a followup, without serious problems. What do
> you think?

It doesn't affect me that much for now, because I only flash variables in the single local scope (because all others are collapsed after a frame step). But I'll need this for bug 751836. Followup.

> > +    // Generate a unique id for the element, if not specified.
> > +    aId = aId || aName.toLowerCase().trim().replace(" ", "-") +
> > this._idCount++;
> >  
> > Should now be
> > aId = aId || aName.toLowerCase().trim().replace(/\s+/g, "-") +
> > this._idCount++;
> > since we can have more than one space and the naming can be more volatile.
> > I'll take care of this in the followup.
> 
> I don't think there can ever be more than one consecutive spaces, but OK.
> 

"Local scope [Something]" has two spaces :)

> > +        // Construct the scope name.
> > +        let name = env.type.substring(0, 1).toUpperCase() +
> > +                   env.type.substring(1);
> > 
> > How about
> > let name = env.type.charAt(0).toUpperCase() + env.type.slice(1);
> > Fits on one line :)
> 
> Tasty! Done.
> 

^_^

> > 
> > +++ b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js
> > 
> > This test does absolutely nothing now :) Maybe remove it and rename the
> > others? Or add something in it, I don't know.
> 
> It still tests the shortening of script labels, so I figured it did
> something useful. I renamed the test function to testScriptLabelShortening,
> in order for that to be more evident.

Ah, ok
Comment 23 Panos Astithas [:past] 2012-05-28 00:15:37 PDT
(In reply to Victor Porof from comment #22)
> (In reply to Panos Astithas [:past] from comment #21)
> > (In reply to Victor Porof from comment #18)
> > > (In reply to Panos Astithas [:past] from comment #16)
> > > > Created attachment 627133 [details] [diff] [review]
> > > > 
> > > 
> > > +            objClient.getPrototypeAndProperties(function
> > > SF_getProps(aResponse) {
> > > +              this._addScopeVariables(aResponse.ownProperties, scope);
> > > +              // Signal that variables have been fetched.
> > > +              DebuggerController.dispatchEvent("Debugger:FetchedVariables");
> > > +            }.bind(this));
> > > 
> > > This makes me a bit worried because there's another signal at the end of the
> > > function. Therefore, we get multiple FetchedVariables events dispatched, but
> > > somebody has no idea what scope they refer to, or if they actually mean "I
> > > got all the variables from all the scopes, you can start testing now".
> > 
> > Since this event was for use by tests only, I opted for triggering one after
> > every protocol response was received, and let the test make sure it got all
> > it needed. I see now that you are counting on it in SF_onFetchedVars, which
> > I hadn't anticipated. It seems to me that making sure FetchedVariables is
> > fired only once can be done in a followup, without serious problems. What do
> > you think?
> 
> It doesn't affect me that much for now, because I only flash variables in
> the single local scope (because all others are collapsed after a frame
> step). But I'll need this for bug 751836. Followup.

Filed bug 759045.

> > > +    // Generate a unique id for the element, if not specified.
> > > +    aId = aId || aName.toLowerCase().trim().replace(" ", "-") +
> > > this._idCount++;
> > >  
> > > Should now be
> > > aId = aId || aName.toLowerCase().trim().replace(/\s+/g, "-") +
> > > this._idCount++;
> > > since we can have more than one space and the naming can be more volatile.
> > > I'll take care of this in the followup.
> > 
> > I don't think there can ever be more than one consecutive spaces, but OK.
> > 
> 
> "Local scope [Something]" has two spaces :)

But not *consecutive* spaces :-)
Comment 24 Rob Campbell [:rc] (:robcee) 2012-05-28 09:57:36 PDT
Comment on attachment 627133 [details] [diff] [review]
Patch v9

send it through the try servers!
Comment 25 Rob Campbell [:rc] (:robcee) 2012-05-28 10:59:11 PDT
one thing I have noticed while playtesting this, the Globals section of the property view do not persist their expanded state across pauses.
Comment 26 Panos Astithas [:past] 2012-05-28 11:30:47 PDT
(In reply to Rob Campbell [:rc] (:robcee) from comment #25)
> one thing I have noticed while playtesting this, the Globals section of the
> property view do not persist their expanded state across pauses.

Right, this is a general issue. We will stop collapsing expanded nodes in bug 751836.
Comment 27 Panos Astithas [:past] 2012-05-28 12:27:52 PDT
Created attachment 627763 [details] [diff] [review]
Patch v10

The patch in bug 758208 broke one test that I fixed. Victor, your patch breaks a bunch more, can you take a look?
Comment 28 Panos Astithas [:past] 2012-05-28 13:44:32 PDT
Created attachment 627780 [details] [diff] [review]
Fix for test breakage

(In reply to Panos Astithas [:past] from comment #27)
> The patch in bug 758208 broke one test that I fixed. Victor, your patch
> breaks a bunch more, can you take a look?

This fixes it for me. Try run:

https://tbpl.mozilla.org/?tree=Try&rev=cc55e4fd061c
Comment 29 Victor Porof [:vporof][:vp] 2012-05-29 04:06:19 PDT
(In reply to Panos Astithas [:past] from comment #28)
> Created attachment 627780 [details] [diff] [review]
> Fix for test breakage
> 
> (In reply to Panos Astithas [:past] from comment #27)
> > The patch in bug 758208 broke one test that I fixed. Victor, your patch
> > breaks a bunch more, can you take a look?
> 
> This fixes it for me. Try run:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=cc55e4fd061c

Looks ok.

For speed, instead of "let prevVar = prevScope && prevScope.children[v];", do a "if (!prevScope) continue;" before the inner loop.
Comment 30 Panos Astithas [:past] 2012-05-29 04:29:29 PDT
(In reply to Victor Porof from comment #29)
> (In reply to Panos Astithas [:past] from comment #28)
> > Created attachment 627780 [details] [diff] [review]
> > Fix for test breakage
> > 
> > (In reply to Panos Astithas [:past] from comment #27)
> > > The patch in bug 758208 broke one test that I fixed. Victor, your patch
> > > breaks a bunch more, can you take a look?
> > 
> > This fixes it for me. Try run:
> > 
> > https://tbpl.mozilla.org/?tree=Try&rev=cc55e4fd061c
> 
> Looks ok.
> 
> For speed, instead of "let prevVar = prevScope && prevScope.children[v];",
> do a "if (!prevScope) continue;" before the inner loop.

OK, I did that, but I forgot to refresh the patch, so it landed separately:

https://hg.mozilla.org/integration/fx-team/rev/d4834de824a9
https://hg.mozilla.org/integration/fx-team/rev/59954c646e78
https://hg.mozilla.org/integration/fx-team/rev/a55eca18fd52
Comment 31 Dave Camp (:dcamp) 2012-05-30 19:28:02 PDT
https://hg.mozilla.org/mozilla-central/rev/d4834de824a9
Comment 32 Dave Camp (:dcamp) 2012-05-30 19:28:28 PDT
and https://hg.mozilla.org/mozilla-central/rev/59954c646e78
Comment 33 Dave Camp (:dcamp) 2012-05-30 19:28:55 PDT
https://hg.mozilla.org/mozilla-central/rev/a55eca18fd52
Comment 34 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-02 05:45:17 PDT
# LOCALIZATION NOTE (scopeLabel): The text that is displayed in the variables
# pane as a header for each variable scope (e.g. "Global scope, "With scope",
# etc.).
scopeLabel=%S scope

I'm a bit lost here. What are the possible values of %S here? I find only a string (globalScopeLabel) in the current files.
Comment 35 Panos Astithas [:past] 2012-06-02 06:07:06 PDT
(In reply to Francesco Lodolo [:flod] from comment #34)
> # LOCALIZATION NOTE (scopeLabel): The text that is displayed in the variables
> # pane as a header for each variable scope (e.g. "Global scope, "With scope",
> # etc.).
> scopeLabel=%S scope
> 
> I'm a bit lost here. What are the possible values of %S here? I find only a
> string (globalScopeLabel) in the current files.

All possible JavaScript scope types, like 'With scope', 'Function scope' or Block scope'.
Comment 36 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-02 06:11:51 PDT
(In reply to Panos Astithas [:past] from comment #35)
> All possible JavaScript scope types, like 'With scope', 'Function scope' or
> Block scope'.

And these "with", "function", etc. are taken from?
Comment 37 Panos Astithas [:past] 2012-06-02 06:22:49 PDT
(In reply to Francesco Lodolo [:flod] from comment #36)
> (In reply to Panos Astithas [:past] from comment #35)
> > All possible JavaScript scope types, like 'With scope', 'Function scope' or
> > Block scope'.
> 
> And these "with", "function", etc. are taken from?

They are specified in the remote debugging protocol:
https://wiki.mozilla.org/Remote_Debugging_Protocol#Lexical_Environments
Comment 38 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2012-06-02 06:25:14 PDT
Which means they'll remain untranslated, isn't it?

Thanks a lot for the explanation, I think this is worth adding to the current localization notes ;-)
Comment 39 Panos Astithas [:past] 2012-06-02 06:53:33 PDT
(In reply to Francesco Lodolo [:flod] from comment #38)
> Which means they'll remain untranslated, isn't it?

That's what I expected, yes. See also the localization note in the top of the file, which mentions that most of these wouldn't make sense if localized, at least in most locales.

> Thanks a lot for the explanation, I think this is worth adding to the
> current localization notes ;-)

OK, I'll make a note to add it in a forthcoming patch that touches this file.

Note You need to log in before you can comment on or make changes to this bug.