Last Comment Bug 771555 - GCLI needs a addon, resize, restart, cookie and pagemanip commands
: GCLI needs a addon, resize, restart, cookie and pagemanip commands
Status: RESOLVED FIXED
[gclicommands]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 16
Assigned To: Nobody; OK to take it and work on it
: developer.tools
Mentors:
Depends on:
Blocks: GCLICMD
  Show dependency treegraph
 
Reported: 2012-07-06 09:22 PDT by Michael Ratcliffe [:miker] [:mratcliffe]
Modified: 2012-07-14 01:36 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Added addon, responsivemode & resizepage (23.21 KB, patch)
2012-07-06 09:22 PDT, Michael Ratcliffe [:miker] [:mratcliffe]
no flags Details | Diff | Review
More commands added (71.27 KB, patch)
2012-07-10 03:57 PDT, Michael Ratcliffe [:miker] [:mratcliffe]
jwalker: review+
Details | Diff | Review
Patch (68.15 KB, patch)
2012-07-10 09:56 PDT, Michael Ratcliffe [:miker] [:mratcliffe]
no flags Details | Diff | Review

Description Michael Ratcliffe [:miker] [:mratcliffe] 2012-07-06 09:22:56 PDT
Created attachment 639699 [details] [diff] [review]
Added addon, responsivemode & resizepage
Comment 1 Michael Ratcliffe [:miker] [:mratcliffe] 2012-07-10 03:57:55 PDT
Created attachment 640560 [details] [diff] [review]
More commands added

Current contents:
- addon
- resize
- restart
- pagemanip
- cookie
Comment 2 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-07-10 04:50:10 PDT
Comment on attachment 640560 [details] [diff] [review]
More commands added

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

Also, we'll need to alter the patch message to reflect the title of the bug.

::: browser/devtools/commandline/GcliCommands.jsm
@@ +307,5 @@
>      threadManager.mainThread.dispatch({
>        run: function() {
>          hud.gcliterm.clearOutput();
>        }
> +    }, Components.interfaces.nsIThread.DISPATCH_NORMAL);

I'm not sure where the Ci->Components.interfaces came from, but I don't think we should be doing that. Happens in several places in this file.

@@ -307,4 @@
>    }
>  });
>  
> -

There's quite a bit of adding and removing blank lines. It's a bit of a nit, but unless there is a reason for it, we should avoid it, and since we're in this patch fixing the Ci/etc and the '.', we should do less LF churn IMHO.

::: browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
@@ +398,4 @@
>  # LOCALIZATION NOTE (cmdDesc) A very short description of the 'cmd'
>  # command. This string is designed to be shown in a menu alongside the command
>  # name, which is why it should be as short as possible.
> +cmdDesc=Manipulate the commands.

Some of the descriptions have periods/fullstops, and some don't. So we need a standard. I vote no periods/fullstops because that's normal in menus.

Makes sense?
Comment 3 Michael Ratcliffe [:miker] [:mratcliffe] 2012-07-10 09:56:10 PDT
Created attachment 640648 [details] [diff] [review]
Patch

(In reply to Joe Walker from comment #2)
> Comment on attachment 640560 [details] [diff] [review]
> More commands added
> 
> Review of attachment 640560 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Also, we'll need to alter the patch message to reflect the title of the bug.
> 

Done

> ::: browser/devtools/commandline/GcliCommands.jsm
> @@ +307,5 @@
> >      threadManager.mainThread.dispatch({
> >        run: function() {
> >          hud.gcliterm.clearOutput();
> >        }
> > +    }, Components.interfaces.nsIThread.DISPATCH_NORMAL);
> 
> I'm not sure where the Ci->Components.interfaces came from, but I don't
> think we should be doing that. Happens in several places in this file.
> 

Changed

> @@ -307,4 @@
> >    }
> >  });
> >  
> > -
> 
> There's quite a bit of adding and removing blank lines. It's a bit of a nit,
> but unless there is a reason for it, we should avoid it, and since we're in
> this patch fixing the Ci/etc and the '.', we should do less LF churn IMHO.
> 

Agreed, removed.

> ::: browser/locales/en-US/chrome/browser/devtools/gclicommands.properties
> @@ +398,4 @@
> >  # LOCALIZATION NOTE (cmdDesc) A very short description of the 'cmd'
> >  # command. This string is designed to be shown in a menu alongside the command
> >  # name, which is why it should be as short as possible.
> > +cmdDesc=Manipulate the commands.
> 
> Some of the descriptions have periods/fullstops, and some don't. So we need
> a standard. I vote no periods/fullstops because that's normal in menus.
> 

Done

> Makes sense?

Ja, alles klar
Comment 4 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-07-10 15:10:04 PDT
https://tbpl.mozilla.org/?tree=Fx-Team&rev=74218d787b06
Comment 5 Tim Taubert [:ttaubert] 2012-07-10 23:45:36 PDT
https://hg.mozilla.org/mozilla-central/rev/74218d787b06
Comment 6 Tim Taubert [:ttaubert] 2012-07-11 02:00:12 PDT
https://hg.mozilla.org/mozilla-central/diff/74218d787b06/dom/base/nsFocusManager.cpp

:( Can you please revert that change? My non-debug build console is getting spammed.
Comment 7 Tim Taubert [:ttaubert] 2012-07-11 02:14:35 PDT
Nevermind, I just reverted it:

https://hg.mozilla.org/mozilla-central/rev/d070bd19c526
Comment 8 Benoit Girard (:BenWa) 2012-07-11 22:46:48 PDT
(In reply to Tim Taubert [:ttaubert] from comment #7)
> Nevermind, I just reverted it:
> 
> https://hg.mozilla.org/mozilla-central/rev/d070bd19c526

Thanks, I was hunting this down :)
Comment 9 Francesco Lodolo [:flod] 2012-07-14 01:03:15 PDT
addonListUnknownHeading=The following addons of the selected type are currently installed:

Not sure about this string: "selected type" and "unknownheading"? When is this label used?
Comment 10 Francesco Lodolo [:flod] 2012-07-14 01:36:16 PDT
pagemodRemoveAttributeRootDesc=CSS selector of root of search
pagemodRemoveElementRootDesc=CSS selector specifying root of search
pagemodReplaceRootDesc=CSS selector to root of search

pagemodRemoveElementSearchDesc=CSS selector specifying elements to remove
pagemodRemoveAttributeSearchElementsDesc=CSS selector of elements to include

Shouldn't these strings use the same structure? If the answer is yes there are probably other strings with the same problem.


Elements matched by selector: %1$S. Replaces in text nodes: %2$S. Replaces in attributes: %3$S.

Replaces->Replacements?



Restarting Firefox...
Should use single unicode character instead of "..."

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