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

2020 08 add pleco link4 firefox android #45

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ascemama
Copy link

@ascemama ascemama commented Aug 6, 2020

Hi Christian,

as discussed a couple of month ago here a pull request which implements a (clickable) pleco image (see attached picture) in the main dialog, which redirect to pleco for the selected word.

  • This feature is only for Android, for other platform nothing is changed. No new behavior/GUI is introduced.
  • (on Android only) I added this as a feature down in the Option tab (see attached picture) so that people can disable it if they don't want to use it.
  • (on Android only) per default the feature is on. (I think most people will be interested to have that)
    -(on Android only) One need to do a long click (keep for ~1second) on the pleco image in order to be able to open in pleco. It seems it is a new behavior, before it was possible to do a normal click. Note that once in pleco it is possible to come back to firefox with the back button.
  • I tested the above described behavior with web-ext on the three platforms Firefox, Chrome, Firefox-Android.

Feedback are welcome ! Hope it will eventually make it to the master branch. Once it is there I will remove my fork from the Addon Firefox store.
Thx,
Antoine

Screenshot_20200806-182324 (1)
Screenshot_20200806-182201

@kondeeza
Copy link

Don't know if you're around but I appreciate this a lot. I just merged this to my own fork and it works great. Nice to have Pleco button at finger tip !

@cschiller
Copy link
Owner

So I tested this with FF 84.1.2 on Android 10 using web-ext and it's not working for me.

What version of FF and Android have you been using for your tests?

@cschiller
Copy link
Owner

BTW, Firefox specific changes should be merged into the firefox branch, not the master branch, which is the one I use for building the Chrome releases.

@kondeeza
Copy link

I can at least confirm that the super old Android 68.1 works fine.

@ascemama
Copy link
Author

ascemama commented Jan 1, 2021

@kondeeza : thx mate :)
@cschiller : sorry I did not write down which version I tested. It was the current version at the time. But anyway, since then Firefox has refactored the android version and there is currently no way to install extensions on the main app except for a few selected extensions. According to Mozilla they will re-allow for installing any extension in the future, but not clear when. You can check this https://blog.mozilla.org/addons/2020/02/11/faq-for-extension-support-in-new-firefox-for-android/

When it will be possible to add extension again I can work on it and submit another pull request (in Android branch) if you are willing to include it. Next time it would be great if you could give me a hint as to when you plan to review and integrate it (or not) so that I know I did not spend time for nothing.

Thx

@cschiller
Copy link
Owner

If it works for you then you didn't exactly spend time for nothing, did you? ;)

I can make no promise as to when, or if, I merge pull requests. But a prerequisite is that they are working and have been properly tested.

I'm willing to look into this again once add-ons can again be installed for Android. The problem seems to be that the event handler that closes the popup is called before the link is being opened. I don't know how to fix this. But, just like you, I don't see much value in spending more time on this if the extension cannot be installed anyway.

So I would suggest to wait and possibly revisit this later.

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.

4 participants