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

fix: undesirable popups when checking for updates #1586

Merged
merged 1 commit into from
May 12, 2020

Conversation

lwouis
Copy link

@lwouis lwouis commented May 11, 2020

Hi!

I use this page for release notes. It contains some JavaScript to load google translate if the user language is not English. This works great in a WebView, however it creates 3 popups:

image

After digging deeper, I found out that the google translate script loads a bunch of iframes. These have no src attribute. They triggers the decidePolicyForNavigationAction method in Sparkle, which calls [[NSWorkspace sharedWorkspace] openURL:requestURL]; which result in the popups.

I don't think there is any point in ever opening a about:blank link for the user, thus this PR ignores these URLs.


If you decide to not merge this, could you please help me build on my fork? I ran make release, then made a zip out of the result, uploaded it to my fork's releases, then made these changes to the podspec. Also note that I had to do that tweak to build on my mac.

After this I updated the Podfile in my project to: pod 'Sparkle', :git => 'https://github.com/lwouis/Sparkle.git', :branch => 'master'.

My project doesn't build and complains that the Sparkle framework is missing. I don't quite understand how Sparkle is packaged/delivered.

Thanks a lot!

Louis Pontoise

… iframes without src attributes, these would make popups appear saying "There is no application set to open the URL about:blank."

A request URL to about:blank has no purpose of being opened, so we ignore it.
@kornelski kornelski merged commit a242035 into sparkle-project:master May 12, 2020
@kornelski
Copy link
Member

Good catch. Thank you

@lwouis
Copy link
Author

lwouis commented May 13, 2020

Hi @kornelski! Thanks for merging the PR!

Do you know how i can get this in my project? I tried really hard to point to master on my fork, but for some reason it doesn't grab the .zip file in my fork releases. Which I built locally on my machine, and uploaded to my fork releases.

I think i'll have the same issue if i point to the main repo master branch because here there is not even a release zip for master i believe.

Are you going to release the next 1.x version soon perhaps? Otherwise i need to be able to point to my fork somehow, but i don't know how to. It just doesn't want to download the zip from my fork even if i remove all the caches in cocoapods. I updated the .podspec file properly I believe, but something is wrong still.

@kornelski
Copy link
Member

I've made an alpha release on github

@lwouis
Copy link
Author

lwouis commented May 14, 2020

Hi @kornelski! Thanks for the release! I'm not sure how to use it though.

How could I use this new alpha release in my project?

@kornelski
Copy link
Member

No, it's not in cocoapods. You can add framework to your project directly, as explained in docs.

@lwouis
Copy link
Author

lwouis commented May 14, 2020

I'm sorry to ask again 🙇‍♂️, but I would really like to avoid ejecting from using cocoapods as I'm doing today. Could you have a quick look at this diff? It's only a few lines. This is what I did when I forked, built Sparkle and locally, and uploaded it on the fork's releases. Is this not enough? What step am I missing to be able to import the project using cocoapods? It think I must be very close and only missing a small step.

@kornelski
Copy link
Member

That looks ok to me. But I've never actually used cocoapods.

@lwouis
Copy link
Author

lwouis commented May 22, 2020

For anyone else that may be struggling in the future, I'm sharing what I did here as the documentation is poor on cocoapods. Here is what I did to get the fix on my project while keeping cocoapods as it was.

pod 'Sparkle', :podspec => 'https://raw.githubusercontent.com/lwouis/Sparkle/master/Sparkle.podspec'

You can't point to the repo using :git. It will ignore the :source directive in the .podspec. you have to point to the .podspec directly. None of this is intuitive or properly documented, so I thought I would share.

Also note that for Google Translate to work, I had to amend the commit I shared in this PR. Maybe Google updated their service or something, because I needed a tweek 1 week later, which is visible in the commit I listed above.

Cheers 👍

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.

2 participants