Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leverage openforge tool for cross-browser extensions #19

Closed
wants to merge 20 commits into from

Conversation

patcon
Copy link
Contributor

@patcon patcon commented May 3, 2014

Following some discussion from #14, I migrated us over to using a tool called OpenForge for building cross-browser extensions. Check out the README for details.

@patcon
Copy link
Contributor Author

patcon commented May 3, 2014

Oh hey, and made some minor css changes to adopt the hidden octicon-heart class that @nathancahill found for his project :)
https://github.com/nathancahill/gittip-button/blob/master/chrome/scripts/main.js#L50
https://github.com/styleguide/css/7.0

(Oh, hey Nathan, your extension is actually broken on Chrome, likely thanks to a Gittip theming change.)

@clone1018
Copy link
Contributor

Can you reformat src/config.json? It's all over the place.

"name": "gittip-everywhere",
"version": "0.0.0",
"authors": [
"Patrick Connolly <[email protected]>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe credit the whole team?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-generated. But yeah sure, my pleasure. I think there are some other places to fix that sort of thing too

@patcon
Copy link
Contributor Author

patcon commented May 3, 2014

Note, this will be relevant once we sort it out: trigger-corp/browser-extensions#16

Currently, we use bower to pull in external libs (just jquery now). We can make sure only the minified js files gets baked into the built extensions.

@patcon
Copy link
Contributor Author

patcon commented May 3, 2014

Hm. Added firefox support. Thought this was done, but hadn't tested twitter because I figured the redesign broken things anyhow. Just realized some old twitter profiles still exist:
https://twitter.com/alexpott

@patcon
Copy link
Contributor Author

patcon commented May 3, 2014

Ah. This was helpful. I think it's the key to getting twitter working
https://trigger.io/docs/v1.4/api/modules/requirements.html

@patcon
Copy link
Contributor Author

patcon commented May 3, 2014

Broke cross compat in the last commit, but can fix it as soon as we figure out why forge.tools.getURL isn't working. likely something about the callback and it not being directly interchangeable with chrome's version

@patcon
Copy link
Contributor Author

patcon commented May 4, 2014

Fixed use of forge for getURL.

but ok, so on firefox, we're apparently doing something wrong, as we can't access Twitter's widget template, nor the icon for Bitbucket.

Seems related to this:
https://stackoverflow.com/questions/17515055/how-can-i-access-firefox-extension-resources

@patcon
Copy link
Contributor Author

patcon commented May 4, 2014

So Chrome seems to be the special snowflake that allows web_accessible_resources, but it's not the norm.

Perhaps we could do templating with this approach, and make sure we play nice with all browser platforms:
http://ejohn.org/blog/javascript-micro-templating/

@patcon
Copy link
Contributor Author

patcon commented May 5, 2014

Hm. This might explain why I've been beating my head against the wall trying to get FF addons to work as expected for Twitter:

honestbleeps/Reddit-Enhancement-Suite#797
https://bugzilla.mozilla.org/show_bug.cgi?id=973750

Also, would love to help extract RES's templating system into it's own package. As I understand it, they have a background script write the templates to localstorage, and can then read that without worrying about weirdness of addons accessing files on browsers like FF. In that extracted lib, we could make the callback used for cross-browser localstorage be tunable, then RES could use their babelext/custom methods, and we could use openforge ones.

cc: @honestbleeps @andytuba @SirCmpwn

@jewel-andraia
Copy link

RESTemplates loads templates.html using a browser-specific "load resource file" function, parses that into a document, and pulls out specific blocks as requested. so, it definitely has to deal with the weirdness of add-ons accessing files =p

@patcon
Copy link
Contributor Author

patcon commented May 5, 2014

@andytuba :)

@clone1018 ok, so as for near-term, I've tested both logged in and logged out for all services:

The only non-conversion related commit was for move the Gittip button on Bitbucket to the left, as it is on Github. (Bitbucket updated their styling and it looked bad where it was on the right.)

@clone1018 I can't think of anything else to do here except review or discuss whether leveraging openforge is something we want to do. Lots of little changes crept in, so if this isn't a decision we can make quickly, then I can strip those out and submit other PR's.

@patcon
Copy link
Contributor Author

patcon commented May 5, 2014

Spoke too soon. Rebased. Done.

@chadwhitacre
Copy link
Contributor

@clone1018 in IRC re: this PR:

I havent looked at it in a while, but it made it super complicated to do simple things.

@chadwhitacre
Copy link
Contributor

The purpose here is to make it easier to have one codebase to support multiple browsers, ya?

@patcon
Copy link
Contributor Author

patcon commented Sep 23, 2014

Yeah, the logic was that so long as our repo looked pretty, it wasn't a big deal that the compiled artifact (ie the extension) looked like garbage. But it seems Mozilla disagrees, so I'd say that puts the nail in this coffin :)

@patcon patcon closed this Sep 23, 2014
@chadwhitacre
Copy link
Contributor

Yikes! Nail indeed. :-/

@seanlinsley seanlinsley deleted the trigger-corp-browser-extensions branch September 23, 2014 17:34
@honestbleeps
Copy link

yeah, Mozilla is hardcore about binaries...

you could still use BabelExt and I'd be happy to help make updates to make your lives easier if you do use it - I don't have a lot of real world users other than myself, so it'd be helpful if others dove in, but if you're not interested I understand!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants