Closed
Bug 814655
Opened 12 years ago
Closed 12 years ago
Re-enable chrome debugging outside of the toolbox
Categories
(DevTools :: Debugger, defect, P2)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 20
People
(Reporter: past, Assigned: past)
References
Details
(Whiteboard: [chrome-debug][fixed-in-fx-team])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Dolske
:
review+
|
Details | Diff | Splinter Review |
The toolbox landing removed the Browser Debugger menu item in favor of a more generic 'Browser Debugging' option that would get all of the tools in the toolbox to target the browser as a whole. However getting other tools besides the JS debugger and the web console to work against chrome will take some time, and even then, only the debugger is plagued with the requirement to launch as a separate process.
For these reasons we decided that it would be best to reinstate the Browser Debugger in its previous form in the meantime, while we work towards getting a more comprehensive solution in place.
Note also that as a workaround, one can already target the browser by launching a separate instance (in a separate profile) and selecting Tools -> Web Developer -> Connect. After connecting to the other browser instance, the list of tabs will be accompanied by a 'Remote Process' option. Clicking on that will open the toolbox with a Global Console and a Browser Debugger.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → past
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [chrome-debug]
Updated•12 years ago
|
Assignee | ||
Comment 1•12 years ago
|
||
Almost there.
Assignee | ||
Comment 2•12 years ago
|
||
This is no new code, just a reversal of a small part of the patch for bug 788977 that brings the Browser Debugger back, for the reasons stated in comment 0. It would be nice to land this in fx-team before we merge the toolbox to m-c so that there won't be any nightlies shipped without the Browser Debugger.
Attachment #684778 -
Attachment is obsolete: true
Attachment #684788 -
Flags: review?(rcampbell)
Attachment #684788 -
Flags: review?(dolske)
Comment 3•12 years ago
|
||
Comment on attachment 684788 [details] [diff] [review]
Patch v2
Review of attachment 684788 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. Going to want a try run to make sure this doesn't introduce any leakage.
Also, the changes to browser.js had to be qref'd a little. There was some problem applying the patch.
::: browser/base/content/browser-appmenu.inc
@@ +155,5 @@
> <menuseparator id="appmenu_devtools_separator"/>
> <menuitem id="appmenu_devToolbar"
> observes="devtoolsMenuBroadcaster_DevToolbar"/>
> + <menuitem id="appmenu_chromeDebugger"
> + observes="devtoolsMenuBroadcaster_ChromeDebugger"/>
I guess you're able to replace the remoteWebConsole entries now because it's take care of by the Connect menu item?
::: browser/base/content/browser-doctype.inc
@@ +19,5 @@
> #endif
> <!ENTITY % aboutHomeDTD SYSTEM "chrome://browser/locale/aboutHome.dtd">
> %aboutHomeDTD;
> +<!ENTITY % debuggerDTD SYSTEM "chrome://browser/locale/devtools/debugger.dtd">
> +%debuggerDTD;
they took it out but you keep putting it back in...
(it's fine. That's what this patch does. I get that, but thought the comment was funny).
::: browser/devtools/debugger/DebuggerPanel.jsm
@@ +93,5 @@
> this._isReady = true;
> this.emit("ready");
> },
>
> destroy: function() {
no more destroy? I guess we can't do that with the devtools window patches applied, can we?
Attachment #684788 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> Looks good. Going to want a try run to make sure this doesn't introduce any
> leakage.
https://tbpl.mozilla.org/?tree=Try&rev=f624f80dc497
> Also, the changes to browser.js had to be qref'd a little. There was some
> problem applying the patch.
Yup, attaching the rebased version.
> ::: browser/base/content/browser-appmenu.inc
> @@ +155,5 @@
> > <menuseparator id="appmenu_devtools_separator"/>
> > <menuitem id="appmenu_devToolbar"
> > observes="devtoolsMenuBroadcaster_DevToolbar"/>
> > + <menuitem id="appmenu_chromeDebugger"
> > + observes="devtoolsMenuBroadcaster_ChromeDebugger"/>
>
> I guess you're able to replace the remoteWebConsole entries now because it's
> take care of by the Connect menu item?
Yes, there is no longer a need for a remote web console entry. I believe it was inadvertently left behind in the big patch and I took the opportunity to clean it up.
> ::: browser/base/content/browser-doctype.inc
> @@ +19,5 @@
> > #endif
> > <!ENTITY % aboutHomeDTD SYSTEM "chrome://browser/locale/aboutHome.dtd">
> > %aboutHomeDTD;
> > +<!ENTITY % debuggerDTD SYSTEM "chrome://browser/locale/devtools/debugger.dtd">
> > +%debuggerDTD;
>
> they took it out but you keep putting it back in...
>
> (it's fine. That's what this patch does. I get that, but thought the comment
> was funny).
<3
> ::: browser/devtools/debugger/DebuggerPanel.jsm
> @@ +93,5 @@
> > this._isReady = true;
> > this.emit("ready");
> > },
> >
> > destroy: function() {
>
> no more destroy? I guess we can't do that with the devtools window patches
> applied, can we?
This was actually another bit of cleanup from my leak-fixing patch in the devtools-window repo. The code in destroy() didn't accomplish anything, I just didn't have the time to remove it before merging.
Attachment #684788 -
Attachment is obsolete: true
Attachment #684788 -
Flags: review?(dolske)
Attachment #685291 -
Flags: review?(dolske)
Assignee | ||
Comment 5•12 years ago
|
||
Try run was green, but for some reason I omitted optimized builds, so I had to run them separately:
https://tbpl.mozilla.org/?tree=Try&rev=3299a44b90ac
Updated•12 years ago
|
Attachment #685291 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Since bug 788977 was backed out, I landed the patch in the devtools-window repo, so that it will be in the updated patch that will reland:
https://github.com/joewalker/devtools-window/pull/317
Assignee | ||
Comment 7•12 years ago
|
||
Landed in fx-team along with bug 788977:
https://hg.mozilla.org/integration/fx-team/rev/c77b869c2025
Whiteboard: [chrome-debug] → [chrome-debug][fixed-in-fx-team]
Assignee | ||
Comment 8•12 years ago
|
||
Landed in m-c:
https://hg.mozilla.org/mozilla-central/rev/c77b869c2025
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•