Closed Bug 839959 Opened 11 years ago Closed 9 years ago

Support devtools and debugger for social components

Categories

(DevTools :: Framework, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: markh, Unassigned)

References

Details

It would be great for potential providers to have access to Fx devtools.

Sadly, it appears the tools aren't really setup for this.  What I've discovered is:

* If you create a "browser" sandbox with the following code, running the sandbox toggles a new devtools toolbox such that the inspector and profiler are targeting the sidebar.

"""
let w = document.getElementById("social-sidebar-browser").contentWindow;
// let w = document.getElementById("social-flyout-panel").firstChild.contentWindow

XPCOMUtils.defineLazyModuleGetter(this, "Toolbox",
  "resource:///modules/devtools/Toolbox.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TargetFactory",
  "resource:///modules/devtools/Target.jsm");
let target = TargetFactory.forWindow(w);
let toolbox = gDevTools.getToolbox(target);
toolbox ? toolbox.destroy() : gDevTools.showToolbox(target, undefined, "window");
"""

Sadly it doesn't also allow the debugger to work.  The best I've come up with here is to manually patch the devtools code.  Hacking dbg-browser-actors.js's onListTabs function to include the following code:

"""
      // Social hacks.
      let browser = win.document.getElementById("social-sidebar-browser");
      let actor = this._tabActors.get(browser);
      if (!actor) {
        actor = new BrowserTabActor(this.conn, browser);
        actor.parentID = this.actorID;
        this._tabActors.set(browser, actor);
      }
      actorPool.addActor(actor);
      selected = tabActorList.length; // pretend the sidebar is selected.
      tabActorList.push(actor);
"""

Does make this work, but obviously it is very hacky, and there isn't a clear way of extending things sanely without patching the code.  Writing an entire new "actor" also seems problematic as the use of the dbg-browser-actor is hard-coded.

I'm not sure how sane any of the above is, but I'm opening this bug to (a) capture the general requirement and (b) keep a note of how far I got before asking for further help :)
One way to use the JavaScript debugger for social components right now is to start a Browser Debugger session, but this is not optimal of course, due to the extra overhead it imposes.

I think content debugging could special-case the social component windows and always add them to the list of tabs (the social-sidebar-browser and all of the social-flyout-panel elements AFAICT). The problem is how to let the user select the debugging target, since we automatically select the current tab as it is. One way this will work right away is to go through the remote connection screen (the "Connect..." menu item), if we provide a way to start a debugger server (bug 832163).

This is not optimal however, so we may want to provide additional UI for choosing the window target in the toolbox. We have such functionality in the debugger already, although hidden by default (bug 806775), which was originally targeted at chrome debugging. Perhaps we should move that inside the toolbox toolbar when social component windows are open.

Would that be enough or would providers like to debug their components even when no windows/panels are open? And the broader question: do we want to treat social components as a form of content or chrome debugging (i.e. treating it as another form of add-on development)?

CCing others who probably have thought about this more than I have.
Thanks for the update and thoughts on how this could be fixed.  One other slightly strange thing is that when I create a toolbox with the code:

 let w = /* ... some browser's contentWindow */
 let target = TargetFactory.forWindow(w);
 let toolbox = gDevTools.getToolbox(target);
 gDevTools.showToolbox(target, undefined, "window");

I notice that the toolbox correctly shows the DOM inspector and profiler are showing the |target|, but the debugger seems to ignore it (and would still ignore it even if we hacked the social elements in as "tabs").  Is that likely to be another bug that would need to be resolved for this?

In the meantime, I've create a new wiki page at https://developer.mozilla.org/en-US/docs/Social_API_Devtools which documents some work-arounds so people can use the devtools in the meantime.
We just had a brainstorming session about this, and the approach we favored was to present these special "debugging" targets in the connection screen (the one you get from the "Connect..." menu item). This screen currently contains input boxes to specify the host and port of a remote instance to connect to, but it would contain an additional list of social components the developer may want to target.

It may seem kind of a stretch to require the developer to "connect" to a target that is available locally, but:
(a) the target group here is quite small (social provider developers vs. mobile/content developers), and
(b) it has the extra benefit of not letting, say, Facebook URLs appear near the URLs of the page one is working on (one of my suggestions in comment 1). There is a significant chance people would be confused and think that the Social API gives Facebook (or anyone else) access to the current tab contents.

(In reply to Mark Hammond (:markh) from comment #2)
> Thanks for the update and thoughts on how this could be fixed.  One other
> slightly strange thing is that when I create a toolbox with the code:
> 
>  let w = /* ... some browser's contentWindow */
>  let target = TargetFactory.forWindow(w);
>  let toolbox = gDevTools.getToolbox(target);
>  gDevTools.showToolbox(target, undefined, "window");
> 
> I notice that the toolbox correctly shows the DOM inspector and profiler are
> showing the |target|, but the debugger seems to ignore it (and would still
> ignore it even if we hacked the social elements in as "tabs").  Is that
> likely to be another bug that would need to be resolved for this?

This is to be expected, since half of our tools can't work with a WindowTarget at all. The API you are currently using will almost certainly change relatively soon, but hopefully after all of our tools have been converted to a single, unified Target API.

It also seems to me that this bug belongs in the Developer Tools: Framework component, unless of course you plan on working on this.
Thanks for the update.  I agree it could make sense for the user to explicitly "connect to" the social components.  I don't quite see how that would interact with adding new actors to dbg-browser-actors.js (ie, I guess somehow it would need to know about this new type of connection, or a brand new "root actor" implementation would be necessary, or something else)

I'm happy to help work on this once an implementation plan becomes clear, but in the meantime I am updating the component as suggested.
Component: SocialAPI → Developer Tools: Framework
We can list all the windows that are not in tabs, and list the URLs + title in the connect screen. That would not be SocialAPI specific though (for example, if the bookmark sidebar is open, we'd see 'chrome://browser/content/bookmarks/bookmarksPanel.xul' listed too).
(In reply to Paul Rouget [:paul] from comment #5)
> We can list all the windows that are not in tabs, and list the URLs + title
> in the connect screen. That would not be SocialAPI specific though (for
> example, if the bookmark sidebar is open, we'd see
> 'chrome://browser/content/bookmarks/bookmarksPanel.xul' listed too).

Purely from a social POV, it would probably be ideal if social was a discrete "target", and a social specific actor (I guess actor - maybe something else) could tell the debugger the <browser> or <iframe> elements suitable for debugging and a title for the element.  Eg, social could them offer "Social Worker", "Social Sidebar", "Social Chat Window 1", "Social Flyout", "Social Toolbar Notification" etc.  This would give more context to the social developer (ie, save them looking at a large list of elements and try to determine manually what is social related) and also avoid the debugger code doing a deep walk of all chrome to find all the relevant elements (eg, the social "chat" targets are inside a custom XBL <chatbar> element, each of while holds XBL <chatbox> elements, each of which has an <iframe> as its only child - it would seem quite painful for the debugger code to generically discover all such elements).

But small steps is also fine - I understand the ideal may take more time than the "good enough" :)
I think social components are a special case, not quite like the bookmark sidebar, which is a traditional target for chrome debugging. Bug 806775 will give chrome developers a better way to concentrate on a particular window in the context of chrome debugging.

My current thinking on the implementation for this includes:

(a) adding a new dbg-social-actors.js file that implements a global-scoped SocialActor, which handles social component window enumeration
(b) SocialActor would implement an onListComponents method, or similar, that would return a list of BrowserTabActor instances
(c) teach browser/devtools/framework/connect/connect.js about this new actor, and have it request and present the list of social components separately from the open tabs

With these implemented, I think all the rest will fall naturally into place.

My reasoning for going with a separate social actor and not extending BrowserRootActor, is that as far as I know the Social API is specific to desktop Firefox, and not something that exists (or is useful) in Firefox OS and Firefox for Android (not to mention other potential future embeddings, like Thunderbird).
Mark, I believe the browser toolbox with it's new ability to switch iframes should address this need.

Is that what you are looking for, or is more work still wanted here?
Flags: needinfo?(mhammond)
The inspector is definitely much better with the iframe switching, however the debugger is quite awkward as well it seems to stop working once I do hit a breakpoint.

- I have to find the page in a long list of scripts in the browser
- Set a breakpoint on an onload handler, reloaded the sidebar, hit the breakpoint, cannot step or run after that

Those may be worth their own bugs rather than pursuing here, what do you think?
Flags: needinfo?(mhammond) → needinfo?(jryans)
(In reply to Shane Caraveo (:mixedpuppy) from comment #9)
> The inspector is definitely much better with the iframe switching, however
> the debugger is quite awkward as well it seems to stop working once I do hit
> a breakpoint.
> 
> - I have to find the page in a long list of scripts in the browser
> - Set a breakpoint on an onload handler, reloaded the sidebar, hit the
> breakpoint, cannot step or run after that
> 
> Those may be worth their own bugs rather than pursuing here, what do you
> think?

Yes, I think specific, focused bugs for these are more likely to see progress.

There is a lot of work going into general polish, and especially breakpoints lately.  So, please do file those issues, and I would expect they will get attention soon.
Flags: needinfo?(jryans)
Depends on: 1152057
Depends on: 1152062
Just going to close this out since it seems the bulk of effort is done and other items are specific bugs.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.