Closed Bug 1243550 Opened 4 years ago Closed 4 years ago

Bring Browser Console to Thunderbird

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: wsmwk, Assigned: Fallen)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 3 obsolete files)

Browser Console is superior to Error Console. But I don't know whether bringing it to Thunderbird is feasible, or even desirable from a long term POV. Ratty and ewong have been working to bring it to SM, so ccing them for comment.

Error Console component is unowned for a long time. Certainly no one in mozilla is going to care about it now that they have Browser Console - and mossop says unclear if he will find an owner.

xref
Bug 587757 - (global-console) Implement Browser Console
https://developer.mozilla.org/en-US/docs/Tools/Browser_Console
https://gist.github.com/mihaisucan/5399023 (Web Console "plan")
Bug 1223341 - Add the Firefox Devtools to the SeaMonkey UI 
Bug 574955 - Make webconsole work in SeaMonkey

Error Console bug reports:
- which mention Thunderbird http://mzl.la/1Uro2P6
- all http://mzl.la/1Uro8WP
I removed usage of the Error console in Firefox (bug 1258985), so support for it will be even more limited. I also suggested droping it entirely:
  https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/XYPqQ58ndX4
I wish we could migrate Thunderbird to Browser console and drop it entirely!

Reposting comment about how to use devtools in thunderbird. I wish I could mentor someone to tackle this.
I can help figuring out devtools issues.

It might not work as-is. But I think it is worth time spent to get the devtools working.
We recently moved devtools codebase out of /browser/.
It now lives in /devtools/. So it is possible to ship devtools into other products!
You can do that simply by setting MOZ_DEVTOOLS=all
  http://mxr.mozilla.org/mozilla-central/source/browser/confvars.sh#70
There might also need some tweaks to package-manifest.in to include the files in the packaged builds:
  http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in#678

Then, most of our codebase is browser oriented. But some parts like the Browser console and Browser toolbox are also meant to work for xulrunner apps. (The names are poorly choosen as they are not browser specific)

Once you managed to ship /devtools/ into your product, you should be able to open a browser toolbox via code like this:
let { loader } = Components.utils.import("resource://devtools/shared/Loader.jsm", {});
// if bug 1247203 *is NOT* in your branch
loader.require("devtools/client/framework/devtools-browser");
// if bug 1247203 *is* in your branch
loader.main("devtools/client/main");

let hudservice = loader.require("devtools/client/webconsole/hudservice");
hudservice.toggleBrowserConsole();

I recently moved the very last bit of devtools from /browser/ to /devtools/ and would be happy to followup in order to ensure our codebase works easily for other products.
I can't promise when I'll get around to this, but given I've done devtools work in the past I am probably the best candidate for this. Once we have a green tree again I can attempt a try run with the variables set. With devtools in its own namespace maybe we can even ship the client.

In any case it would be a great exercise to see if it is truely independent of Firefox.
Intent to remove Error Console has been posted https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/XYPqQ58ndX4/discussion
Blocks: cc-backlog
See Also: → 1223341
It seems the Error console is also at least partly broken in Thunderbird trunk atm. E.g. alert("foo"); + Evaluate doesn't do a thing anymore. Doesn't even givn an error in the Error console ;)
Fallen, how green do the trees need to be? I think we are generally OK now at least on OS X and Windows. Also this looks like it would be immediately visible whether the new console is working or not.
Flags: needinfo?(philipp)
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
I've done a build with this and it at least does not fail. I'm checking what else needs to be done to get the browser console opened.
Are people using the Error Console as-is in Thunderbird?  According to Comment 5, it's not fully functional.  Trying to gauge if I should hold off on removing the code in Bug 1278368 until this is further along
(In reply to Brian Grinstead [:bgrins] from comment #7)
> Are people using the Error Console as-is in Thunderbird?  

definitely, and essential. fallen is pretty familiar with Browser Console, so if anyone can get it to work he can. But some time is needed.
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
That wasn't so bad. I haven't gone through extensive testing, please test to see if it works just as well for you.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7d46d150d888

There are some hardcodings in m-c I could work around, but ideally I should find time to fix the hardcodings. I've discussed with jryans that it would make sense to introduce a chromeWindowType property in https://dxr.mozilla.org/mozilla-central/source/devtools/client/framework/devtools.js similar to how it already exists for the devtools server. It would also make sense to pass that to the addBrowserActors() call there as well. Mentioning this in hopes that I will find it in case I forget.
Assignee: nobody → philipp
Flags: needinfo?(philipp)
Attachment #8760984 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED
Is webconsole-overlay a new file?  It seems to be missing from the patch if so.
Flags: needinfo?(philipp)
Attached patch Fix - v2 (obsolete) β€” β€” Splinter Review
Yep, just noticed I missed that one too, thanks for the quick reply.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=dc989ce11a91
Attachment #8760984 - Attachment is obsolete: true
Attachment #8760984 - Flags: review?(mkmelin+mozilla)
Flags: needinfo?(philipp)
Attachment #8760993 - Flags: review?(mkmelin+mozilla)
(In reply to Philipp Kewisch [:Fallen] from comment #11)
> Created attachment 8760993 [details] [diff] [review]
> Fix - v2
> 
> Yep, just noticed I missed that one too, thanks for the quick reply.
> 
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=dc989ce11a91

You may want to restart (push?) the job again since there seems to be an infrastructure problem, and
all compilation jobs failed...

TIA
Attached patch Fix - v3 (obsolete) β€” β€” Splinter Review
Here is a try run with fixed packaging:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=dcca4d7f5bac
Attachment #8760993 - Attachment is obsolete: true
Attachment #8760993 - Flags: review?(mkmelin+mozilla)
Attachment #8761085 - Flags: review?(mkmelin+mozilla)
tested try build, windows XP opt. LGTM
Comment on attachment 8761085 [details] [diff] [review]
Fix - v3

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

If I run with -jsconsole, trying to click any links of where errors occur, I get 

this.gViewSourceUtils is undefined

::: mail/components/devtools/content/webconsole-overlay.xul
@@ +4,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<!DOCTYPE overlay SYSTEM "chrome://messenger/locale/messenger.dtd">
> +
> +<overlay id="tb-webconsole--overlay"

single "-" please :)
(In reply to Magnus Melin from comment #15)
> Comment on attachment 8761085 [details] [diff] [review]
> Fix - v3
> 
> Review of attachment 8761085 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> If I run with -jsconsole, trying to click any links of where errors occur, I
> get 
> 
> this.gViewSourceUtils is undefined

It's trying to first open in a browser window, then second falling back to gViewSourceUtils: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/hudservice.js#438.  What is the desired behavior for these links in Thunderbird?
We opened a standalone View source window with the file in question and scrolled to the source line in question. Also the matching line was selected. Any number of such windows could stay open.
The current Error Console seems to be using the same module, although loaded as a script in the console.xul doc: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/console/content/consoleBindings.xml#541, and https://dxr.mozilla.org/mozilla-central/source/toolkit/components/console/content/console.xul#27.

Whereas this is reaching up to the top level window to get a reference to it, so you might need to include a script reference to viewSourceUtils.js in the top level window.
imho I don't think opening view source pages in tabs would be a problem, if that's easier to implement.
With the wonky tab implementation we have, it might not be that nice to have it in a tab. 

To be clear, opening it from the menu does't have the problem, it's only with "-jsconsole"
The this.gViewSourceUtils undefined error only shows up with -jsconsole and will be fixed once I patch devtools to not rely on navigator:browser. See also the workaround I installed when opening it via menu. The problem is that it uses the webconsole.xul window when browserWindow is null, which doesn't have gViewSourceUtils. One thing that just came to mind is we could include the jsm for gViewSourceUtils in the webconsole overlay I added, this might be the better workaround.
Attached patch Fix - v4 β€” β€” Splinter Review
This should do it. I intentionally did not add viewsourceutils to the main window this time, so that when I fix devtools to work well with mail:3pane instead of navigator:browser this will break instead of silently work, forcing me to remove the workaround.
Attachment #8761085 - Attachment is obsolete: true
Attachment #8761085 - Flags: review?(mkmelin+mozilla)
Attachment #8761855 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8761855 [details] [diff] [review]
Fix - v4

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

LGTM, r=mkmelin
Attachment #8761855 - Flags: review?(mkmelin+mozilla) → review+
https://hg.mozilla.org/comm-central/rev/4da79677dea3 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
Depends on: 1279874
Added a missing semi-colon (r=Fallen over IRC)
https://hg.mozilla.org/comm-central/rev/8c9f1c7a0889
Could you backed out this from Earlybird52(next release TB)?
Because, It is difficult to execute debugging of addon due to Bug 1279874.
Flags: needinfo?(philipp)
It is not possible to back this out since the original error console has been removed from the Mozilla Codebase.
Flags: needinfo?(philipp)
You need to log in before you can comment on or make changes to this bug.