burningmoth / burningmoth-chromelogger-firefox

WebExtension implementing Chrome Logger protocol for Firefox
15 stars 12 forks source link

Hello, thanks, and... PRs? #10

Open mpaperno opened 2 months ago

mpaperno commented 2 months ago

Hi Tarraccas,

First of all, many thanks for this extension! I finally found a replacement for the old FireLogger extension/protocol I've used "forever," but that extension got deprecated ages ago. I work with ColdFusion back-ends a lot, and made a server-side extension for that, which I've now adapted for ChromeLogger (and vice versa).

So, I've made a bunch of changes (hopefully for the good :), and was just wondering if you're interested in integrating. I pushed everything I have so far to a dev branch on my fork (that dev branch is the default).

I'm happy to break things up into PRs as much as possible, if you want, or whatever is best for you. Of course pretty quickly my commits build on top of each other because they touch the same code. But a lot of the features/fixes/changes can be applied individually with minimal code adjustment (if you only want parts, for example).

Some highlights:

(See commit notes on comparison for more.)

I threw in some screenshots below of the console output of my test page with a bunch of logging (though still a fraction of the total output/data possible now), and options page, just for reference.

Anyhow, if you're not interested or don't use this anymore, I totally understand, no worries. Just wanted to run it by you. I'll put up a couple releases on my fork (once I figure out the signing thing). Of course all the protocol-level changes wouldn't help anyone until server-side libs are adjusted anyway... not sure if anyone still uses this stuff or not! :-)

Thanks again! -Max

Firefox:

image

(The blue debug line styling and icon are set with a userContent.css, as is the ability for the backtrace to overlap/hide the usual JS backtrace on the right.)

Chrome:

image

(Note quite as nice since it is more restrictive in which style attributes are allowed, and forces inline-block display, etc.)

Options panel in FF

image

(Dark mode, showing all default options.)

tarraccas commented 2 months ago

Hi Max.

Appreciate your taking the time to pitch this impressive and intriguing proposition. As chance would have it, I was furiously working on an update to this extension (3.0, Mozilla approved it late last week) while you were also composing your message. So, among other features, I've happened to add dark theme and console.debug() method while I was fixing the reason it'd stopped working for me: eval'd code stopped working for JSON viewer wrapped pages which was the fallback for when the content script failed to inject. Mozilla reviewers had gotten pissy with me over the eval'd code (even though their API allows for it) years ago so I guess I shouldn't be too surprised that they eventually got around to blocking it under the pretense of security without providing any additional access to the web console in that time but I digress. And I hadn't noticed this because I was stuck on Firefox 113 on an outdated Ubuntu 18 installation. That is, until I upgraded and noticed the errors in the PHP code (that I'm also upgrading) aren't showing up in the web console anymore. I've spent the last month trying to find workarounds (there aren't any) before finally surrendering myself to developing an entirely separate devtools panel with which to display messages.

A little background on how this extension came to be. Firefox (40-something I think) had built-in support for the ChromeLogger protocol. So, like you, I built a small library to catch and route PHP errors to the web console. That's all I needed and so all I wanted to ever build. Well, Firefox 57 rolls around with its Add-On-Pocalypse and this useful feature is removed and that just won't do. So I frantically threw together this extension (with no prior experience and old JS skills) and added the substitution styles that I discovered were a thing in the process of doing so. Got some feedback, squashed some bugs and called it good. That's all I needed and so all I want ever build ... until nearly 5 years later this last month. I'm of the opinion that a good tool does its job and then gets out the way and I really wanted to keep this simple, minimal so I'm a tad annoyed that a front-end had to be added. I'm even more annoyed that upgrading it to manifest 3 will require an intrusive permission system in order to continue working. But, again, I digress.

Craig who developed the ChromeLogger protocol also maintained a Chrome extension at last check which is one of the reasons I never bothered to support Chrome. In fact, there's a FirePHP extension that supports the ChromeLogger protocol on Firefox and Chrome as well. I just didn't care for its UI or that it doesn't send messages to the web console. Do you think this extension adds anything to Chrome that these or other extensions that support the protocol don't already offer?

Could use the help but I gotta wrap this up here for now. Just wanted to let you know that I got your message.

mpaperno commented 1 month ago

Hi Tarraccas,

Good to hear from you. Quite a coincidence with the timing!

The most important to me are the protocol changes I've made. I'm not sure how anyone really used this on the server side w/out knowing the extension was even "listening." And being able to process multiple headers is key for me.

Since my initial post here, I discovered that Craig's original PHP library and his plugin would inject/handle multiple X-ChromeLogger-Data headers already. So messages can be injected into headers at any point in the response processing instead needing to accumulate them. The issue preventing the same behavior with the Firefox version was actually in how FF delivers the header values -- in Chrome multiple headers with same name are delivered individually, whereas in FF they're all delivered as one string, with commas separating the values of each individual header. So sending multiple -Data headers didn't work at all, which was a deal-breaker for me.

I've fixed that (and kept ability to split long messages to multiple headers, also a key point for me). I've also added a way to send flags to the server for filtering messages or adding additional details, and a way to provide a password to the server as well. Also added ability to send a timestamp with each log row. Basically this brings it up to parity with the Firelogger "protocol" features I used to use.

I've seen FirePHP, but it was already dead IIRC. Funny how much it looks like Firelogger. I don't know any details of the protocol. The extension code is convoluted.

I actually briefly started my own panel also... but because I wanted more control over the output formatting. I decided it wasn't worth it... for now. But I see you implemented a lot of it already... even token parsing. Cool! And w/out any external libs... even cooler!

I'm not entirely sure I understand the reason a fallback is needed... or parsing the source for a script with the data? I mean, if there's a script in the output already, couldn't it just log whatever is needed? Or any output for that matter.... For me the biggest advantage of this remote/header method is being able to debug "ajax" requests which may not even output any HTML at all.

Under what circumstances does the log.js script fail to inject? I'm just curious since I've not come across that.

But yea in terms of formatting and other possibilities, a custom output console would be cool. Get rid of the useless local timestamp, log location and stack traces and stick our own in there. I'd like an "object inspector" view like the Firelogger/FirePHP sidebar which is nicer than expanding stuff inline.

As for Craig's Chrome plugin... it seems dead. There are issues filed asking for some of the protocol stuff I've implemented, and even a PR for injecting a "beacon" header into the request. All seems ignored. Reviews seem to suggest it's not working, but I haven't tried it. There's a PR Craig filed for MV3 update, but unless that gets merged, the plugin will be altogether unusable sometime next year. And I don't see anyone taking it over.... 🤷🏼 I figured I'd post something over there once this new version that works on Chrome is squared away, and I'd probably publish it on the Chrome market as well. If Craig shows interest, I'm happy to work with him on the protocol additions.

Anyway, I pushed some more commits to my fork with the changes described above. Some of it is just housekeeping, and some is Chrome specific. If you're interested, I can re-base some/all of this on your latest version (skipping what you're already covered). It will come out cleaner that way since I'll be able to squash more stuff together. In any case, you may want to look at the UTF8 fix (6767a8) and know that FF doesn't populate the proper defaults in nested objects when using storage.sync.get() (1c5293).

I'll need to re-work the header injection for Chrome MV3 (blocking a webRequest to add header no longer allowed), but that should work for FF also. Or at least FF 128+. I'm not sure which FF even started supporting MV3. But at least 128 is an LTS release.

Cheers, -Max