ssborbis / ContextSearch-web-ext

Search engine manager for modern browsers
329 stars 37 forks source link

Add the shortcut to toggle Firefox sidebar #559

Closed wordpure closed 1 year ago

wordpure commented 1 year ago

Fix #541.

Add a shortcut to toggle the Firefox sidebar and make it easier for users to see and close the search results in the sidebar.

ssborbis commented 1 year ago

This modification to the manifest causes the addon to fail to load in chrome-variants. Firefox and chrome do have separate manifests that I've instructed users to swap out if sideloading, depending on their browser, but the default manifest did work ( albeit with errors ) in Chrome. This may be better suited to place in the firefox_manifest.json vs the main manifest.json for that reason. When I build a release, FF will get the changes, and chrome users can still pull the source and sideload with the default manifest.

wordpure commented 1 year ago

This modification to the manifest causes the addon to fail to load in chrome-variants.

Chrome doesn't have the sidebar, so this command doesn't work with Chrome.

Firefox and chrome do have separate manifests that I've instructed users to swap out if sideloading, depending on their browser, but the default manifest did work ( albeit with errors ) in Chrome. This may be better suited to place in the firefox_manifest.json vs the main manifest.json for that reason.

I only modified firefox_manifest.json, which was used to replace the manifest.json file for testing purposes.

图片

If the errors you see when testing are the ones in the screenshots, it may be that Chrome reads the default manifest.json when loading the extension directory.

图片

The manifest.json in the current repository does contain some inappropriate Chrome's configuration.

图片

When I build a release, FF will get the changes, and chrome users can still pull the source and sideload with the default manifest.

When I looked at the source code of the extension you submitted to AMO and the Chrome Web Store, I found that AMO contains both manifest.json and firefox_manifest.json, while the Chrome Web Store only has manifest.json (should be created by chrome_manifest.json was renamed). Is there any special reason here?

图片

图片

Only firefox_manifest.json is modified in the PR, if you submit it to AMO in the original way, you'll have to do additional processing.

ssborbis commented 1 year ago

The manifest.json in the current repository does contain some inappropriate Chrome's configuration.

This is somewhat intended. That manifest.json can be used by both Firefox and Chrome if sideloading using the source code from the repository. Chrome throws errors, but it will load the extension.

When I looked at the source code of the extension you submitted to AMO and the Chrome Web Store, I found that AMO contains both manifest.json and firefox_manifest.json, while the Chrome Web Store only has manifest.json (should be created by chrome_manifest.json was renamed). Is there any special reason here?

My build script for chrome may remove the browser-specific manifests, leaving only the copy intended for chrome. Not sure why the Firefox build keeps it. Just an oversight. It shouldn't effect anything.

Only firefox_manifest.json is modified in the PR, if you submit it to AMO in the original way, you'll have to do additional processing.

I appreciate your thoroughness in testing. I do run separate build scripts that will modify the manifests to be compliant with the intended browser. Today, I modified the Firefox script to include your changes.