Building my own version of the code - problems

gingerbeardman commented 5 years ago

I would like to contribute some changes in the future.

But when I build an unedited fork of the code it does not work correctly.

app binary is generated OK
extension can be activated OK
extension context menus appear OK
extension context menus do not function BAD
extension context menus disappear after use BAD

Any ideas why this might be?

I am signing with my own details in Xcode.

yimingliu commented 5 years ago

it sounds like some of the context menu logic haven't been properly set up in validateContextMenuItemWithCommand and contextMenuItemSelectedWithCommand (especially the disappearing one -- the current validateContextMenuItemWithCommand is designed to hide menu items that aren't search-google or search-bing), but without seeing the code, it's hard to say.

If you'd like, feel free to fork this repo and push your changes to your fork / branch, and I can take a look when I get a chance.

gingerbeardman commented 5 years ago

I've not yet made any changes to your code. I should have made that clear, sorry!

I figured I'd just compile it and confirm it was all working in my build before I began editing.

yimingliu commented 5 years ago

Oh. Uhh, I don't know. I've checked in all the files that I have here, and I was able to build on a new machine (just today -- to reproduce the previous problem in #2).

When the context menu shows up but doesn't work, it usually means it's not able to communicate to the extension process. Safari spins up a separate process for every extension; if that extension interprocess communication is not working for some reason, or if that extension process is stalled, etc., that's usually what happens. But I don't know...I'd have to be able to reproduce this somehow.

The first thing I'd check is if there are leftover processes or ReverseImageSearch.app on that hard drive that isn't your custom-built version. Safari handles multiple versions of the same extension very badly, as I've learned (even between dev versions and the release version I put up in Releases)
Second thing I'd check is to see if there are any useful logs. If you're building the extension and launching the app, the logs should go to Console.app. If you're launching the extension directly in XCode, it should be in the log section of the IDE (but sometimes isn't -- 'cause for whatever reason it could fail to attach, and you'd have to restart the IDE, and it's a huge hassle and I still don't know why it does that)
If all else fails, you could also make your own Safari App Extension project from scratch, and just copy over the .js file, the .plist file(s), and the .h/m files from ReverseImageSearch, turn on Hardened Runtime and try to install that. ReverseImageSearch has no external dependencies so this should be a straightforward copy. If that works but my repo's project doesn't, then there must be some kind of setting difference -- at least that narrows it down?

gingerbeardman commented 5 years ago

OK, some progress...

The context menus not appearing seems to be a conflict with another script I am using (now removed!) on every page. From: https://safari-extensions.apple.com/details/?id=cf.mattijs.smartrightclick-7G36RSR4E4

So, now I can get the context menus to appear all the time.

But, they don't do anything when I click them. Neither does clicking "Open in Safari Extension Preferences..." from the app.

gingerbeardman commented 5 years ago

When I reinstalled your version, all the missed context menu clicks were handled at once! A bunch of pages popped up.

Will keep looking. I think a signing issue?

gingerbeardman commented 5 years ago

If all else fails, you could also make your own Safari App Extension project from scratch, and just copy over the .js file, the .plist file(s), and the .h/m files from ReverseImageSearch, turn on Hardened Runtime and try to install that. ReverseImageSearch has no external dependencies so this should be a straightforward copy. If that works but my repo's project doesn't, then there must be some kind of setting difference -- at least that narrows it down?

This has solved the issue! Now to find the difference.

BTW: I did not turn on hardened runtime, so that did not fix the earlier issue.

gingerbeardman commented 5 years ago

The only differences I can find are the signing section.

Search Images.zip

Xcode project folder
Hardened runtime is ON, but it doesn't make a difference
I called my new project "Search Images" I doubt that would make a difference

yimingliu commented 5 years ago

Sorry for losing track of this ticket -- day job and all. Gonna poke around more on this issue over this weekend, but as you might imagine, it's hard to fix something I can't easily reproduce -- and I only have 1 developer subscription to work with.

gingerbeardman commented 5 years ago

Yeah no worries, I have been using the version I built locally and forgot all about the issue myself!

gingerbeardman commented 4 years ago

Built OK for me just now. Closing.

yimingliu / reverse-image-search-safari

Building my own version of the code - problems #3