Port to WebExtension (WIP) #176
Conversation
Cimbali
force-pushed
the
master
branch
2 times, most recently
from
f8c9b0d
to
9a132ea
Compare
|
actually why supplement over forking as new project? @diegocr doesn't seem to have any spare time |
|
@alekksander We'll see in due time. I'm trying to make this work decently first, then hopefully discuss with @diegocr, and if he indeed doesn't want to maintain the project any more I could take it over. |
Basically only whitespace and renaming variables and functions. Should be functionally completely equivalent to previous commit.
- Add a manifest for webext version and remove install.rdf - Remove files & functions that do setup (bootstrap.js, ...) - Keep functions that do actual cleaning work (browser.js -> cleanlinks.js)
Remove recursive functions, as content scripts are injected in every frame.
Add messaging between background and content script to send cleaned-click events and numbers of links cleaned. In background.js, update the count (on the badget text), notify the user, and (TODO) track the cleaned links for potential future whitelisting
Loading options and updating+storing options on change still to be done.
Requires storage permission. Options are stored as they are displayed in the options page, and loaded based on the object types in the default values. All the main code is wrapped in the loadOptions.then() to be sure that we load options from storage before anything else. A message is sent to the background script to trigger a new loadOptions() whenever new values are saved, so that it stays up to date.
|
Anyone who wants to test the web-extension, please grab web-ext, clone this PR and run Alternately, go to |
|
Can you make an xpi of it for those of us not familiar with the extension building? |
|
i'm afraid tere's no xpi's anymore. You don't even have to build anything, just get the source code and follow one of the two ways described above: Get the source: If you have npm, the node.js package manager: Alternately, use the debugging mode:
That's really all there is to it. |
Also repeat the dirty hack to handle links added in a document with ajax, which consistss of running a clean again after repdelay seconds after a click.
A new contextMenu item is required for this, as we can't intercept the
previous one anymore in web extensions.
The setup for this feature is a little contrived:
- copying to clipboard can not be done from a background script.
- on links or selections, the target text (link to be cleaned) is
available from the background script where the contextMenu event is
handled.
- otherwise, only the script in the page knows where the right click
that triggered the contextMenu is, thus what the text is.
Thus, organise the flow thusly:
1. In injected content script, on right click, send text link around
click position to background and wait for (optional) response.
2. In background script, if our contextMenu item is clicked, get sent
text link and clean it (or use a.href or selected text), and send
cleaned link as message reply
3. In injected content script, if there is a response to the message,
copy it to the clipboard.
NB. Are we leaving hanging promises around? Or do they get GC'd?
NB2. There might be a race condition when accessing lastRightClick, i.e.
the saved text link message (content script (1.) -> background (2.)).
If clicked too fast, maybe the message didn't arrive yet. To fix
that, we probably would need to wrap lastRightClick in another
promise.
There is a browser preference that controls this: "When you open a link in a new tab, switch to it immediately" However this preference is not available through the browserSettings API: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserSettings
|
„no xpi's anymore”? |
|
WebExtensions are a new API and format for extensions, and the whole reason this addon has to be ported in the first place. WebExtensions are notshipped as xpi's, just as is or zipped (usually via addons.mozilla.org) |
|
well i can at least confirm that this branch does work. I am enjoying it in 59.0.3 x64 win10 |
|
@Cimbali can you post this on AMO ？ |
|
I will publish on AMO as soon as this is at beta-level of readiness. Specifically I want to implement all features before putting this out there, so I still want to do the cleaning of |
|
i confirm it's working fine under fx59 on ubuntu based system. could translate to polish if needed, and waiting for the official release to start complaining/suggesting things :) |
Also move the cleaning function to cleanlinks.js.
Perform the cleaning in the onRequest function by returning the
{redirectUrl: ...} object if we have a cleaned link.
Integrate as comments all fixes from the original XUL code in case those
bugs show up again.
The AMO parser was throwing a syntax error on parsing the previous version for some reason. Logic remains the same: identify a link in a jacascript:... text and keep just that.
|
Alright, the webextension fork is out on AMO! Welcoming bug reports and feature requests at https://github.com/Cimbali/CleanLinks/issues Get the add-on here: https://addons.mozilla.org/addon/clean-links-webext/ |
This is a port of CleanLinks to web extensions.
A few ways of cleaning links have already been implemented:
I'm starting this PR so that anyone who wants to help, see the progress, or otherwise comment on this work can do so. In particular I think I didn't use most of the features of the add-on, so I'm not entirely sure I'm rebuilding it correctly.
The way this is built, is
cleanlink.jsis included everywhere and contains all the common code.background.jsruns the background "service,"inject.jsis a content script that is injected in every frame, andoptions.jsis run on the options page.