1157906 - Inspect element button is not showing up in the Browser Toolbox or in WebIDE

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P1)

Description

[Brian Grinstead [:bgrins]]

Attachment: browser-toolbox-checkbox.png

Just noticed this today - the Inspect Element button isn't showing up in the Browser Toolbox.  It's 'checked' in the Options panel, and shows up if I toggle it again but it's missing on startup.

Comment 1

[Brian Grinstead [:bgrins]]

More info here: https://bugzilla.mozilla.org/show_bug.cgi?id=1128988#c15 and here: https://bugzilla.mozilla.org/show_bug.cgi?id=1128988#c16.

Comment 2

[Joe Walker [:jwalker]]

For symmetry with main.js, I did https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/Loader.jsm#82, so I guess the loader used for the browser toolbox doesn't respect this for some reason.

Comment 3

[Brian Grinstead [:bgrins]]

Copying in comments from other bug:

Somehow this has caused the Inspect Element button to disappear in the Browser Toolbox (Bug 1157906).  Looks like it may be a loader issue?

Module `definitions` is not found at resource://gre/modules/commonjs/definitions.js

"@resource:///modules/devtools/commandline/commands-index.js:82:22GcliActor<._getRequisition@resource://gre/modules/devtools/server/actors/gcli.js:245:22GcliActor<.specs<@resource://gre/modules/devtools/server/actors/gcli.js:96:12actorProto/</handler@resource://gre/modules/devtools/server/protocol.js:1006:19DSC_onPacket@resource://gre/modules/devtools/server/main.js:1456:15DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/devtools/transport/transport.js:471:9makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.js:82:14makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.js:82:14"

Comment 4

[Brian Grinstead [:bgrins]]

FYI, to run the Browser Toolbox in a separate window so you can see logs, you can open up a normal BT on  so that the server starts and then run a separate command:

./mach run --profile /tmp/foobar -chrome chrome://browser/content/devtools/framework/toolbox-process-window.xul --purgecaches --jsconsole

Comment 5

[Joe Walker [:jwalker]]

What happens if we do:

  const defaultTools = require("resource:///modules/devtools/definitions.js").defaultTools;

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #5)
> What happens if we do:
> 
>   const defaultTools =
> require("resource:///modules/devtools/definitions.js").defaultTools;

I'm seeing this error after doing that (it seems to be clipped off in browser console though):

onPacket threw an exception: Error: Server did not specify an actor, dropping packet: [{"item":"command","name":"pref","params":[],"isParent":true,"description":"Commands to control settings","manual":"Commands to display and alter preferences both for GCLI and the surrounding environment"},{"item":"command","name":"tilt","params":[],"isParent":true,"description":"Visualize the webpage in 3D","manual":"Investigate the relationship between various parts of a webpage and their ancestors in a 3D environment","hidden":true},{"item":"command","name":"tilt open","params":[],"isParent":false,"description":"Open the Inspector 3D view","manual":"Initialize the 3D page inspector and optionally highlight a node using a CSS selector","hidden":true},{"item":"command","name":"tilt toggle","params":[],"isParent":false,"hidden":true,"buttonId":"command-button-tilt","buttonClass":"command-button command-button-invertable","tooltipText":"3D View"},{"item":"command","name":"tilt translate","params":[{"na

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: gcli-error-log.txt

Here's the full log from Comment 6

Comment 8

[Brian Grinstead [:bgrins]]

OK so I think the loader thing was a mistake in my first test - I probably just hadn't fully built before running the BT process (or the cache hadn't been properly cleared, or something).  I think Comment 7 is the actual problem

Comment 9

[Joe Walker [:jwalker]]

GcliActor is a child of the tab actor, that feels like it could cause this?

Comment 11

[Joe Walker [:jwalker]]

I just duped bug 1158248 to this because there were significant similarities: "onPacket threw an exception: Error: Server did not specify an actor, dropping packet", but it's possible they don't actually have the same root cause.
If when we fix this, the other isn't fixed, then we should undupe it.

Comment 13

[Brian Grinstead [:bgrins]]

This is also affecting simulators connected via WebIDE:

onPacket threw an exception: Error: Server did not specify an actor, dropping packet: {"error":"unknownError","message":"Error occurred while creating actor 'undefined: TypeError: browser is undefined\nStack: exports.GcliActor<.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/gcli.js:1:952\nconstructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js:16:80\nObservedActorFactory.prototype.createActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/common.js:13:99\nDebuggerServerConnection.prototype._getOrCreateActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:83:53\nDSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:93:11\nChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/transport/transport.js:38:337\nLine: 1, column: 951"}
Stack: DebuggerClient.prototype.onPacket@resource://gre/modules/devtools/dbg-client.jsm:944:1
DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:471:9
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14
Line: 944, column: 0

Comment 14

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I'll attempt to see if I can fix this.

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: 0001-Bug-1157906-Can-t-return-arrays-as-a-root-response-f.patch

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=423f990a8f43

Comment 16

[Brian Grinstead [:bgrins]]

Comment on attachment 8598292 [details] [diff] [review]
0001-Bug-1157906-Can-t-return-arrays-as-a-root-response-f.patch

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

LGTM!

Comment 17

[Marcos Caceres [:marcosc]]

Nit, why not just: 

 return this._getRequisition().then(
   requisition => requisition.system.commands.getCommandSpecs(customProps)
 );

Comment 18

[Tim Nguyen :ntim]

(In reply to Marcos Caceres [:marcosc] from comment #17)
> Nit, why not just: 
> 
>  return this._getRequisition().then(
>    requisition => requisition.system.commands.getCommandSpecs(customProps)
>  );

I saw this notation was deprecated somewhere on MDN.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Marcos Caceres [:marcosc] from comment #17)
> Nit, why not just: 
> 
>  return this._getRequisition().then(
>    requisition => requisition.system.commands.getCommandSpecs(customProps)
>  );

Just to remove the return?  That's fine too, but also seems like an uber style nit to me. :) Either way, the patch I've attached is still needed.

Comment 20

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #18)
> (In reply to Marcos Caceres [:marcosc] from comment #17)
> > Nit, why not just: 
> > 
> >  return this._getRequisition().then(
> >    requisition => requisition.system.commands.getCommandSpecs(customProps)
> >  );
> 
> I saw this notation was deprecated somewhere on MDN.

Ah, it's just for the classic function notation : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Expression_closures

Works for arrow functions.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/fd84063e1296

Comment 22

[Ryan VanderMeulen [:RyanVM]]

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

Comment 23

[Joe Walker [:jwalker]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #19)
> (In reply to Marcos Caceres [:marcosc] from comment #17)
> > Nit, why not just: 
> > 
> >  return this._getRequisition().then(
> >    requisition => requisition.system.commands.getCommandSpecs(customProps)
> >  );
> 
> Just to remove the return?  That's fine too, but also seems like an uber
> style nit to me. :) Either way, the patch I've attached is still needed.

FWIW, the style guide in my head says that the (x => y) format is for one liners, and I do

    (x => {
      return y;
    })

When it goes onto more than one line. But I'm not fussy about it.

Comment 24

[Marcos Caceres [:marcosc]]

As an aside, we should really get our (Moz) act together and adopt a machine-processable style guide. The amount of variation from one team to another wrt JS code makes things really hard to work with and review. 

I wonder where we can have this conversation?

Comment 25

[Michael Henretty [:mikehenrty][:mhenretty]]

jryans, when should I see this fix in nightly? I still don't see my inspect button in the latest nightly.

40.0a1 (2015-04-30)

Comment 26

[Brian Grinstead [:bgrins]]

(In reply to Michael Henretty [:mhenretty] from comment #25)
> jryans, when should I see this fix in nightly? I still don't see my inspect
> button in the latest nightly.
> 
> 40.0a1 (2015-04-30)

I double checked and I do see the button showing up in Browser Toolbox but am still seeing the issue in WebIDE with the following error:

"Protocol error (unrecognizedPacketType): Actor server1.conn0.child8/gcliActor11 does not recognize the packet type specs" protocol.js:20
onPacket threw an exception: Error: Server did not specify an actor, dropping packet: {"error":"unknownError","message":"Error occurred while creating actor 'undefined: TypeError: browser is undefined\nStack: exports.GcliActor<.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/gcli.js:1:952\nconstructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js:16:80\nObservedActorFactory.prototype.createActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/common.js:13:99\nDebuggerServerConnection.prototype._getOrCreateActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:83:53\nDSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:93:11\nChildDebuggerTransport.prototype.receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/transport/transport.js:38:337\nLine: 1, column: 951"}
Stack: DebuggerClient.prototype.onPacket@resource://gre/modules/devtools/dbg-client.jsm:944:1
DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:471:9
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14
Line: 944, column: 0 DevToolsUtils.js:58:0
"Protocol error (unrecognizedPacketType): Actor server1.conn0.child8/gcliActor11 does not recognize the packet type specs" Promise-backend.js:870

Comment 27

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Michael Henretty [:mhenretty] from comment #25)
> jryans, when should I see this fix in nightly? I still don't see my inspect
> button in the latest nightly.
> 
> 40.0a1 (2015-04-30)

It seems there are still unsolved issues remaining for both old servers of any kind (bug 1160345) and new non-Firefox servers (bug 1160361), unfortunately.

Follow those for the remaining work.