-
Notifications
You must be signed in to change notification settings - Fork 3
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
[WIP] Refactor background, increase version, remove permission #2
Conversation
This PR also relates to 1 since the PR introduces a fetch from the |
Thank you very much for your suggestions, we are currently also interested in improving the addon and we will have a look at it. |
Just to make it clear @gremid, my assumption is that the PR uses the polyfill as we just planned: |
@adbar I also spotted the "Building a cross browser extension" on MDN. In this article at the very top is a note, stating, that this is for Manfiest v2. I've also tried to use When f.e. the polyfill is used, the |
After some local testing, I see that I have no problems with Chrome and Opera (suggestions enabled) but with firefox. Immediately after the refactoring of the add-on, there were no problems. With firefox only the context menu works correctly. The |
I've
Building a add-on version for firefox and one for chrome/opera/edge can be done with the following script: cd dwds-addon/src
rm -R web-ext-artifacts/
web-ext build
mv web-ext-artifacts/im_dwds_nachschlagen-0.1.4.zip ../im_dwds_nachschlagen-0.1.4-firefox.zip
mv manifest.json manifest-firefox.json
mv manifest-chrome.json manifest.json
web-ext build
mv web-ext-artifacts/im_dwds_nachschlagen-0.1.4.zip ../im_dwds_nachschlagen-0.1.4-chrome.zip I've pushed a parameterized (version) script to the PR. Here is what you get when running |
Thanks for the changes, feel free to add an Let's give everyone time to review the PR if necessary and aim for a release next week. At least for Firefox I can test and update the existing extension. |
Ok, the newest commits are now also in this PR included.. Issues resolved.. |
@adbar did you get the omnibox to work with firefox? (@dwds + Space in the address bar) I have no idea what to use instead of omnibox. The API seems to be used correctly and Firefox version 113 is supported. In the For an example usage of the omnibox API see here |
No, the omnibox is not working consistently in the current version so it is no downgrade if it isn't working after your changes. |
… manifest and add-on version;
One other thing I tried to get the suggestions to work in firefox is the Like you already stated, this is not a downgrade, since the add-on is fully working for chrome and opera. Here is a bugzilla issue about the omnibox API. |
@Mastercuber Thanks, I was able to build and test the extensions, the build script is really helpful. There are still parts missing, here what I expected and what I found, feel free to complete or comment. I'm using the current add-on as a reference, the working functions are shown by the screenshots.
Before releasing a new version for Firefox I hope we can solve the right-click issue. Maybe I did something wrong with the tests on Chromium, the add-on builds and installs correctly but the functionality currently isn't there (at least on my computer). Is that normal? |
When I change the keywords (omnibox = dwds1; search_provider = dwds2) I can't find the search engine anywhere in the settings, only the It seems like the
Right clicking on a word is not enough, since the context menu item only gets added when text is selected (docs). For me it seems like it would be possible somehow to implement this functionality, but with a non small time consumption.. Processing the selected text, instead of figuring out, what text is actually meant to be searched with DWDS is certainly the better way to go.
In firefox the keyword specified in So in firefox - since both keywords are the same and the search provider keyword gets prefixed with Like mentioned in section 1, in chrome the search_provider seems to get ignored. The omnibox API is working (also with suggestions)
Section 2 should clarify this. |
Thanks for the details, here is the new status according to my tests:
I'll merge this PR and publish a new Firefox version, for Chrome I'll need to go through the registration process first. |
This pull request refactors the background script, so that this can be used in firefox and chromium based browsers like Chrome, Opera, Edge.
Also a manifest file for deploying as a chrome extension is created witrh this PR.
The version of the manifest itself is increased to 3. In the README file you can find an answer to why using v3.
In the README also links to the Chrome Webstore and how to publish to the Webstore was added.
I have tested functioning of the plugin successfully with firefox and chrome. Opera is not working with manifest version 3 for now. Another manifest for Opera should do the trick, but seems a bit strange, since another manifest is needed for version 3 but one is enough for version 2.
All in all this PR refactors the background script, increases the manifest version, removes unnecessary permissions and add some Chrome webstore and manfest version related links to README.
To load the plugin manually into the browsers for testing, you can use:
chrome://extensions/
about:debugging#/runtime/this-firefox
opera://extensions
For Chrome and Opera the manifest file must be renamed to
manifest.json
and the current one be stashed or removed.