771555 - GCLI needs a addon, resize, restart, cookie and pagemanip commands

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Added addon, responsivemode & resizepage

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: More commands added

Current contents:
- addon
- resize
- restart
- pagemanip
- cookie

Comment 2

[Joe Walker [:jwalker]]

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

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 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]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=74218d787b06

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/74218d787b06

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

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] (inactive)]

Nevermind, I just reverted it:

https://hg.mozilla.org/mozilla-central/rev/d070bd19c526

Comment 8

[Benoit Girard (:BenWa)]

(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]]

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]]

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 "..."