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

Safari Extension API #110

Closed
Techdojo opened this issue Oct 19, 2017 · 26 comments
Closed

Safari Extension API #110

Techdojo opened this issue Oct 19, 2017 · 26 comments

Comments

@Techdojo
Copy link

Following on from comments on #65
The library should support the safari native messaging api to enable the library to be used in classic style Safari extensions.

@vhmth
Copy link
Collaborator

vhmth commented Nov 2, 2017

🙌

@tshaddix
Copy link
Owner

@Techdojo Happy to review a PR for this :)

@mmarsella
Copy link

Has anyone begun development on a Safari implementation? I would gladly contribute.

@vhmth
Copy link
Collaborator

vhmth commented Jan 15, 2018

Not that I know of @mmarsella. That would be incredible and greatly appreciated!

@mmarsella
Copy link

mmarsella commented Feb 8, 2018

@tshaddix @vhmth I have written a "safari-react-redux" implementation of your library. I'm not sure how it would stand as a PR vs. submitting it as a stand-alone library for Safari extensions - as it is a pretty heavy makeover from Chrome, thoughts?

One idea would be to check in wrapStore / Store.js if window.chome or Safari/etc... and point to that specific browser implementation. But that would definitely be a change on the chrome end as well.

@Techdojo
Copy link
Author

Techdojo commented Feb 9, 2018

@mmarsella I'd be interested in having a look at your PR, I've finally been able to get back on to working on my own Safari implementation and like my Edge solution (which works like a dream once you get past all the "other" Edge extension issues) is being implemented as a chrome.runtime polyfill.

I'm setting up up my own Safari message passing solution and then "emulating" the connect and sendMessage functionality.

@mmarsella
Copy link

@Techdojo I'll create a repo of my implementation for you to check out w/ a simple sample project. I am still confused as to whether or not this should be a stand alone library (crediting the original authors of course) vs. trying to have my implementation extend react-chrome-redux (which would be somewhat mis-leading).

@mmarsella
Copy link

@Techdojo @vhmth @tshaddix

Here is a repo with an example of how my safari implementation works. Please let me know your thoughts. Again, I'm glad to publish as a standalone library or modify it to be a PR (see above thread).

https://github.com/mmarsella/safari-react-redux

Hope to hear from ya'll soon.

M

@Techdojo
Copy link
Author

@mmarsella - thanks, I'm having a look through it now :)

@Techdojo
Copy link
Author

@mmarsella - thanks for uploading your code, it's definitely a lot cleaner / simpler approach than the chrome polyfill approach I took (shame as that worked well on Edge).

I've had more success implementing your method, the only thing I needed to do was jury rig requiring your code if running on Safari as opposed to the other browsers (I'm using a single codebase for all versions).

One thing I did notice was in your Store.init() method you're making a call to safari.self.tab.dispatchMessage(), am I right in thinking that this is correct for scripts that are injected into the page as I'm only using this library to communicate between my popover and the background and it was throwing an error.

@mmarsella
Copy link

@Techdojo - I'm glad you like it and thanks for checking it out!!!

Nice catch on the popover functionality... my example only deals with the background page and injected/content scripts (hence why safari.self.tab has been working for me) but I did not account for popovers. I think I just need to make another check to see in the init() to see if we are coming from a popover (maybe in the "from" parameter when initializing the Store).
I'll revise and push some changes up.

I'm also going to restructure the repo to export the library how 'chrome-react-redux' does, and keep the example repo in there as well.

I have been mulling over as to whether or not I should have this be additive to the react-chrome-redux library. if that was the case, I was thinking about using dynamic imports after doing some browser sniffing. From what I gathered, determining which browser we are in isn't clear cut.

Thoughts on this vs. it being a stand alone library?

@Techdojo
Copy link
Author

My gut feeling is that I there's enough differences behind the scenes in the way that Chrome and Safari implement their message passing schemes that it justifies a 'safari-react-redux' specific version. Having tried (and failed) to come up with a clean generic solution I think it'll be easier with two separate libraries that implement the core interface. That way you also avoid having to bake in any nasty browser detection or setup code to choose which set of functions to use, plus it prevents bloat on any users who don't need or care about the Safari version.

Interestingly Apple seem to be deprecating the ideas of Javascript based Safari extensions in favour of Safari App extensions that are written in ObjC / Swift, use UIKit (for popovers) and are packaged in native MacOs apps and that can be distributed via the App store.

@tshaddix
Copy link
Owner

@mmarsella Awesome work on this. Thanks for putting it together and sharing it. The only significant downfall I can see from pulling it into its own library is losing future updates/features such as the middleware support currently in PR.

As @Techdojo mentioned - it's definitely going to be easier to pull it into its own package. Really your call on this one!

@Techdojo
Copy link
Author

@mmarsella - I just found this which you might find interesting.
https://stackoverflow.com/questions/12663743/how-do-i-pass-a-message-from-a-safari-extension-popover-to-the-global-page?rq=1

https://developer.apple.com/library/content/documentation/Tools/Conceptual/SafariExtensionGuide/AddingPopovers/AddingPopovers.html#//apple_ref/doc/uid/TP40009977-CH21-SW1

Basically - you don't pass messages from popovers to the global page, you just make reference to 'safari.extension.globalPage.contentWindow' and from there can call any function you like on the background page. I'll do some more experimenting and see if I can get it working.

@Techdojo
Copy link
Author

Ok so this works.

if you want to send a message from the popover instead of from an injected script - In Store.init replace
if (window.top === window) { safari.self.tab.dispatchMessage('proxyInit', { port: this.portName, type: 'proxy-init', proxyStoreID: this.id, href: document.location.href, from: this.from, }); }
with
safari.extension.globalPage.contentWindow.capturePopoverMessage('proxyInit', { port: this.portName, type: 'proxy-init', proxyStoreID: this.id, href: document.location.href, from: this.from, });

And then in wrapStore.js add the following

window.capturePopoverMessage = (name, message) => { handleMsg({ name, message, target: { url: 'popover' } }); };
Immediately after
safari.application.addEventListener('message', handleMsg, false);
And then you'll be able to receive messages from either source.

I'll post again when I've got a clean method for responses going back the other way.

@vhmth
Copy link
Collaborator

vhmth commented Feb 15, 2018

🎉 beautiful - thank you for keeping us posted as you jump down this rabbit hole @Techdojo!

@Techdojo
Copy link
Author

Techdojo commented Feb 16, 2018

Coming back the other way - the global page needs to get the popover window context like so

        const popoverWindow = safari.extension.popovers[0].contentWindow;
        popoverWindow.captureMessageFromBackground(details);

Then in popover.js you need to declare the function on the window that will receive the data - something like this

let store;

window.captureMessageFromBackground = (msg) => {
        store.handleMessageFromBackground(msg);
}

store = new Store({ portName: 'SafariReduxPort' });

I added an accessor method (handleMessageFromBackground()) into Store.js so I can 'pass' the message in - this is basically the function that's registered as the handler for "message" events.

The really fun part was realising and ensuring that this function is declared BEFORE you create an instance of the store class as the sending of (and receiving the response from) the 'proxyInit' message is handled in the Store constructor and therefore still in the context of that function and as the function hasn't technically finished yet and any other code after it hasn't been executed yet - gotta love me some Javascript :)

Quick - addendum...

The above still doesn't work as because the constructor function hasn't finished the store variable is still undefined, so the only option is to extract out the this.init() call from the store constructor, so that the constructor can finish, the store variable can be set AND then you can call store.init();

@mmarsella
Copy link

@Techdojo Nice - thanks a ton for diving in on this - very much appreciated. I've been tinkering with adding popover functionality as well and I like your approach. I should have something up by this evening.

@mmarsella
Copy link

mmarsella commented Feb 17, 2018

@Techdojo I just pushed up a cleaned up "library-format" version of react-safari-redux. If you happen to figure out how to include popover functionality, would you submit a PR to the react-safari-redux repo here: https://github.com/mmarsella/react-safari-redux.

I changed the example repo I attached earlier in this thread to this: https://github.com/mmarsella/react-safari-redux-example.

^^ This uses the react-safari-redux library via npm.

I commented out the popover in the gulpfile, which can be easily be added back in by un-commenting any popover related tasks. Hope this helps as a solid boilerplate for testing popover features : )

Thanks a ton - and I'll try to chip away at a popover solution as well.

Maybe in regards to any Safari - related conversation / issues, we can move to the safari repo?

@tshaddix Please let me know if there is anything you would like me to include in my repo that I may have left out. Thanks again for all of the support!

@tomjohn1028
Copy link

@Techdojo have you gotten anywhere with this? I'm currently porting an existing chrome extension to safari, but given Apple's push to deprecate the Safari Extensions for Safari App Extensions they're moving further and further away from the WebExtensions API. It makes it pretty difficult to keep a single code base depending on architecture and functionality.

Note: I don't currently use the react-chrome-redux architecture (might transition in a couple months), but generally curious about any progress on Safari App Extensions you all are making.

@Techdojo
Copy link
Author

@tomjohn1028 - I was able to get my extension up and working (and has been live for a few months now) using this method. I never got round to merging my code back into the branch - my solution ended up differing a bit too much and I only implemented the basic features required for the popup window to be able to communicate with the background page - let me know if you'd like a copy of what I've got working so far.

@vhmth
Copy link
Collaborator

vhmth commented Jun 15, 2018

@Techdojo based on what you've worked through, would you say it'd be worth it to get react-chrome-redux workable within the Safari ecosystem? It's looking like 2 things are working against us:

  1. Discontinued support for extensions from Apple
  2. An API/setup for Safari extensions that differs enough to make it so your changes aren't mergeable back in here.

Let me know if I'm jumping the gun here. If that isn't the case and it's still possible to get this workable with the Safari extension ecosystem, that would be great news.

@tomjohn1028
Copy link

@vhmth @Techdojo I'm jumping down the rabbit hole of the new Safari App Extensions that Apple is pushing developers to. There are a lot of missing features or differences that most likely make it not mergeable (e.g. no background page, background is handled by a process written in native languages). Happy to share what I've come across if anyone is making the transition.

@vhmth
Copy link
Collaborator

vhmth commented Jun 18, 2018

Ah I see @tomjohn1028. Good to know. If there's no background process in JS (assuming it's swift), the usefulness of this lib kinda goes out the window. Will wait on @Techdojo to confirm my last comment before closing.

@Techdojo
Copy link
Author

@vhmth @tomjohn1028 I'm desperately trying to avoid going down the app extension route, it's a massive overhead for what amounts to such a small portion of our userbase - especially seeing as we already have a fully working solution, for me the benefits don't outweigh the implementation cost.

That said - I think it should be possible to get a working solution integrated in to the main branch, but it's basically webextension for Chrome & FireFox (& Edge with a very simple polyfill) and Non standard for Safari but a fork is probably an easier route.

Also I can confirm having looked at the Safari App extension - that you basically create a native MacOS app using XCode, then add the Safari App extension target (which generates some boilerplate example code). You then write the app extension using Swift (or Obj-C if you really want) and then use some really crappy messaging protocol to talk to the UIKit view controller (for the popup - so no easy way to get React running there) and the content scripts for the content injection part. Your injected scripts are bundled and injected as normal, it's everything else about the implementation that's changed.

Also with the App Extension there are still a LOAD of missing features and bugs as well as extra hoops to jump through as @tomjohn1028 mentioned. You wouldn't believe the pain I had just getting a basic JSON file to be downloaded from our server and returned as an NSDictionary.

@vhmth
Copy link
Collaborator

vhmth commented Jun 21, 2018

Given that, I'm closing this. Thank you for your investigation to everyone here. Y'all rock!

@vhmth vhmth closed this as completed Jun 21, 2018
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

No branches or pull requests

5 participants