Merge Chrome and Firefox implementations #19
Conversation
| @@ -1,12 +1,12 @@ | |||
| { | |||
| "name": "zipkin-browser-extension", | |||
| "version": "0.0.0", | |||
| "version": "0.0.4", | |||
There was a problem hiding this comment.
In the future bump this to change version instead of VERSION file.
npm version is great for it (creates a git commit as well as a tag)
SimenB
changed the title
|
@adriancole @eirslett This can be reviewed & (hopefully) merged. I'd like to add some tests before release, but I'm also gonna do a small reorganizing of the files, and then git thinks it's all new files. So (squash) merging this separately makes for a more accurate history 🙂 |
| "persistent": true | ||
| }, | ||
| "icons": { | ||
| "48": "icon64.png" |
There was a problem hiding this comment.
This shows an icon, so sorta #2. Would like that SVG though
|
fyi "npm run dev-chrome" or firefox doesn't work (might be unrelated) |
|
It's just |
|
was able to test chrome via your instructions However, FF doesn't seem to like the build directory (when I go to install add-on from file) |
|
decided on gitter to handle FF separately |
| @@ -2,7 +2,8 @@ export default function addNetworkEvents(network, pubsub) { | |||
| network.onNavigated.addListener(() => { | |||
| pubsub.pub('navigated'); | |||
| }); | |||
| network.onRequestFinished.addListener(request => { | |||
| // https://bugzilla.mozilla.org/show_bug.cgi?id=1311171 | |||
| network.onRequestFinished && network.onRequestFinished.addListener(request => { | |||
There was a problem hiding this comment.
For posterity, this is why FF doesn't work yet
I uploaded the resulting zipfile for Chrome to Mozilla's online check for compat (https://www.extensiontest.com/).
So this PR ports all of the build from gulp to webpack, and deletes all of the firefox code.
I still want to add add https://github.com/mozilla/webextension-polyfill which allows us to not use any chrome APIs directly. We can also simplify much of the code which is written to abstract over the browser differences. The polyfill also gives us Promise APIs, which another chunk of the code in this repo also does.Done