Cimbali / CleanLinks

Converts obfuscated/nested links to genuine clean links.
https://addons.mozilla.org/en-GB/firefox/addon/clean-links-webext/
Mozilla Public License 2.0
76 stars 2 forks source link

Can't open links in new tab #22

Closed nerone closed 4 years ago

nerone commented 6 years ago

While using extension it's not possible to open links in new tab when using keyboard shortcut Cmd/Ctrl + link. Reproducible on google.com.

Cimbali commented 6 years ago

Works for me, both with scroll-button click and ctrl + click. even the "switch to tab" option

Can you be more specific? CleanLinks version, Firefox version, platform? Thanks

Imold commented 6 years ago

I can confirm that middle click and control left click both open the link in the current tab, not in a new tab. Browser: Waterfox 56.2.0 (64-bit)

Cimbali commented 6 years ago

Interesting. You have outgoing HTTP requests cleaning disabled, is that right?

It appears if you activate it the request gets denied, and on a second click it then works. Both work fine on firefox 60.0.

Cimbali commented 6 years ago

Turns out we use the openerTabId property which is compatible with ff 57+, to indicate to which tab to return on closing the new tab. We'll probably need to figure out which is the lowest version we want to support and account for that.

nerone commented 6 years ago

Did a bit of testing. I can still reproduce this using v3.0.3 on clean Firefox 60.0.2 install on OS X 10.11.6. But I have tried on Windows and it works as expected.

Cimbali commented 6 years ago

Can you maybe open the Console (Ctrl + Alt + Shift + i, it's going to ask for remote debugging confirmation) to check for error messages?

Basically on the toolbox that the shortcut opens, go to the Console tab, purge the history for ease of reading (trash can in the top left corner), and click a link that fails.

There should be an error message from either cleanlink.js, background.js or inject.js which are the 3 CleanLinks scripts involved.

nerone commented 6 years ago

I tried add-on debugging console, but it shows nothing related to Clean Links extension when I Cmd + click on links in Google search results.

Cimbali commented 6 years ago

Ah, my bad, the error doesn't show because there already is a fallback in case of error. I've added a print, in the debug_print_22 branch.

I don't have a mac to reproduce this, so what would be helpful to debug this issue would be for you to get that branch with the debug print, and report the error.

Here's a step by step guide:

Now just repeat the steps to look at the debugging console while doing Cmd + Click and an error message should appear. You can filter the console by "Logging > Errors" using the buttons in the top row.

nerone commented 6 years ago

clean-links

I get only this in console when Google search results loaded and nothing else when Cmd + clicking on links.

Cimbali commented 6 years ago

Alright, well that's a start. Maybe that exception in the background script prevented it from correctly opening a new window later on or something. I fixed it, now let's see if it works.

nerone commented 6 years ago

Now that error has gone and no other appear.

nerone commented 6 years ago

@Cimbali, can you please re-open the issue, if it’s not already fixed?

Cimbali commented 6 years ago

@nerone My main problem with this issue is just that I can't reproduce it. It is fixed for Waterfox and FF < 57 though, just not your setup (maybe os x is the troublemaker?)

Bt7fdbufffdk commented 5 years ago

this issue also occurs on reddit when clicking on any of those types of links shown in the screenshot

sfdgchvjb
Cimbali commented 4 years ago

I think this is actually a really simple bug now I look back at it. We recreated the ctrl + click behaviour manually:

https://github.com/Cimbali/CleanLinks/blob/dbcdb4ff53841c681607223c2a40f0c1c529dac0/addon/inject.js#L37-L38

On macOS the default is to use cmd + click instead of ctrl + click. So now we check for both. There should not be too much interference from meta + click on other OSs, as this is not a meaningful combo usually.

Some testing on macOS still needs to happen, because ctrl + click should open the context menu, so we maybet accidentally overwrite that behaviour. Ideally I would like not going down the rabbit hole of detecting which OS we are running on to adapt behaviour etc.

Cimbali commented 4 years ago

I can confirm this now works as expected.