Firefox: Simulate default action for clicking links with link hints #2602

mrmr1993 · 2017-08-15T19:06:53Z

This makes the LinkHints.activateModeToOpenInNew{,Foreground}Tab commands work as expected on Firefox.

Currently the flag (simulateClickDefaultAction) is hardcoded on, which opens tabs twice in Chrome for the affected cases. It could be an option, use UA sniffing, or both (default set by UA sniffing), so I've left it for now.

@smblott-github this is a big usability win (currently FF can only open links in the current page).

philc · 2017-08-17T18:30:08Z

Any reason to delay merging this?

mrmr1993 · 2017-08-17T18:33:19Z

simulateClickDefaultAction needs to be set differently on the FF side vs. the Chrome side, so a decision needs to be made on how to do that. I'll push a UA sniffing commit here shortly.

AFAIC, this is otherwise mergeable. I don't like that we have to do it or how it's done, but it does work.

philc · 2017-08-17T18:48:51Z

Sorry, I didn't read the PR description carefully enough. Thanks for clarifying. Yes, we need browser-branching before merging =)

mrmr1993 · 2017-08-17T18:53:02Z

Pushed UA sniffing for Firefox (as Utils.isFirefox). This should now be ready to go 🎉

I added a small change that gets rid of the lag when reloading the extension on FF; I can extract it out if need be. See comment on the commit for more.

gdh1995 · 2017-08-24T10:51:55Z

background_scripts/main.coffee

@@ -5,6 +5,7 @@ root = exports ? window
 chrome.runtime.onInstalled.addListener ({ reason }) ->
  # See https://developer.chrome.com/extensions/runtime#event-onInstalled
  return if reason in [ "chrome_update", "shared_module_update" ]
+  return if Utils.isFirefox()


Why to do nothing on runtime.onInstalled if the browser is Firefox?

Re-injecting our content scripts doesn't work properly (ie. doesn't activate Vimium in tabs before they've been reloaded) on Firefox, and it makes the browser lag considerably while it's doing it. It was annoying me while testing, so I disabled it.

Oh, it does fail. Thanks! Then I want to test the update notification if you haven't, in order to ensure nothing is working on Firefox.

test the update notification if you haven't

I haven't. Go for it.

mrmr1993 · 2017-08-27T17:04:15Z

@smblott-github @philc can this be merged? I've been using it (on Chrome and Firefox) for nearly 2 weeks without any problems.

@mrmr1993

- Remove the low-level test which examines intividual modifier settings and replace it with a higher-level test which lists the relevant modes. The previous test was hard to read. - Expand the test to open *any* `href` (not just for `a` elements). Can this do any harm? Mention @mrmr1993.

smblott-github · 2017-08-29T06:50:54Z

What about like this, @mrmr1993? The test of the modifier keys looked too confusing to me.

gdh1995 · 2017-08-29T18:49:26Z

I think there may be some websites which don't call preventDefault for a mouse event on <a href="#">function...</a>, and neither do those on <a href="javascript:;">...</a>, so it's better to check link.getAttribute("href") not in ["", "#"] and "://" in link.href.

href="#" and herf="javascript:;" are often used to provide a underline for the link text and make an <a> tab-focusable.

mrmr1993 · 2017-08-30T00:11:31Z

Good call! I wondered about these, but assumed they would spawn a new window. I've certainly never come across a spec for it not.

ivanruvalcaba · 2017-09-06T16:48:24Z

Hi all,

I don't know if the following information is relevant but I have noticed that this functionality doesn't work properly if you change the user-agent (tested on my Firefox Nightly 57.0a1, Linux 64bits host). I am currently using the AgentX web extension for that. This happens when I replace the user-agent with Google Chrome on MS Windows, so if I have to use this functionality I must restore the user-agent to a default value.

Best regards.

smblott-github · 2017-09-07T05:22:14Z

this functionality doesn't work properly if you change the user-agent

That's a serious problem. Could we figure out the browser on the background page and then inform the front end when each frame registers?

gdh1995 · 2017-09-07T05:45:51Z

Why not check typeof browser === "object" && !!browser.runtime?

mrmr1993 · 2017-09-07T12:09:55Z

Could we figure out the browser on the background page and then inform the front end when each frame registers?

Done. I've used a method which should be fairly future-proof and shouldn't break if/when Edge and Chrome adopt Mozilla's extension API additions.

mrmr1993 · 2017-09-07T12:17:27Z

I think there may be some websites which don't call preventDefault for a mouse event on <a href="#">function...</a>, and neither do those on <a href="javascript:;">...</a>, so it's better to check link.getAttribute("href") not in ["", "#"] and "://" in link.href.

href="#" and herf="javascript:;" are often used to provide a underline for the link text and make an <a> tab-focusable.

@gdh1995 I've just done some testing on this. The current behaviour matches what clicking does with the relevant modifier keys (except for javascript: URLs, where Firefox won't let us open a tab). I don't think these are changes we should make.

gdh1995 · 2017-09-07T13:20:12Z

The current behaviour matches what clicking does with the relevant modifier keys

OKay.

And the new API runtime.getBrowserInfo does work. Good job!

ghost · 2017-09-21T10:15:32Z

@mrmr1993 thanks for this fix! Thanks to it I can complete my switch from Chrome to Firefox without missing anything.
@philc Do you think that after fixing the merge-conflicts, this PR can be merged?

smblott-github · 2017-09-21T12:06:53Z

content_scripts/link_hints.coffee

+            clickActivator = (modifiers) -> (link) ->
+              defaultActionsTriggered = DomUtils.simulateClick link, modifiers
+              simulateClickDefaultAction = Utils.isFirefox()
+              if simulateClickDefaultAction and


@mrmr1993. We can merge this. But I'd really appreciate it if you could find a way of avoiding repeating the modifier/new-tab logic here. That logic rightly belongs -- in my opinion -- with the various modes at the top of this file.

What do you think about moving it into DomUtils.simulateClick? Since it's a workaround for that rather than specifically for link hints, there makes more sense to me.

I'll rebase and make the change -- whichever you choose -- in a little bit.

mrmr1993 · 2017-09-21T21:12:59Z

I've rebased this, moved the fix code into DomUtils, (hopefully) made the code a bit clearer, and added an equivalent workaround for the shift key modifier.

now simulate the default action to open a new tab during VHints.*.run if needed merged: * philc/vimium#2602 * philc/vimium#2681 differences: * not deal with the case of "shift=true,others=false" * it's impossible during normal processes * support <a rel~=noopener>

mrmr1993 mentioned this pull request Aug 17, 2017

FF nightly: Shortcut F to open new tab doesn't work #2603

Open

smblott-github added the smb-watching label Aug 17, 2017

fturcheti mentioned this pull request Aug 22, 2017

"Open link in foreground/background tab" not working in Firefox lusakasa/saka-key#21

Closed

gdh1995 reviewed Aug 24, 2017

View reviewed changes

mrmr1993 mentioned this pull request Aug 27, 2017

Firefox popup blocker blocks link opening #2612

Closed

smblott-github mentioned this pull request Sep 1, 2017

[Firefox] Following link hints in a new tab does not work #2625

Closed

mrmr1993 mentioned this pull request Sep 12, 2017

Exclude Firefox from the scaled DPI fix #2637

Merged

smblott-github reviewed Sep 21, 2017

View reviewed changes

mrmr1993 added 2 commits September 21, 2017 20:35

FF: Simulate default action for clicking links with link hints

e6a529e

Use Utils.isFirefox for link hints fix

a8337e9

mrmr1993 force-pushed the ff-link-hints-new-tabs branch from e8649b9 to a8337e9 Compare September 21, 2017 19:39

Add openUrlInNewWindow command

7fa22da

mrmr1993 force-pushed the ff-link-hints-new-tabs branch from 315d5a1 to a8337e9 Compare September 21, 2017 20:54

Move simulating click default action to DomUtils, add shift handling

92d2b40

naclander mentioned this pull request Sep 24, 2017

Firefox: opening link in new tab ('F' button) do not work. #2510

Closed

smblott-github merged commit f0c90f3 into philc:master Sep 25, 2017

This was referenced Sep 26, 2017

Firefox: Cannot open links in the body of Medium posts #2672

Closed

FF: Simulate default click action for <a target="_blank">s #2681

Merged

mrmr1993 mentioned this pull request Oct 15, 2017

[Firefox] Shift-F behavior triggers Firefox pop-up blocker #2726

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firefox: Simulate default action for clicking links with link hints #2602

Firefox: Simulate default action for clicking links with link hints #2602

mrmr1993 commented Aug 15, 2017

philc commented Aug 17, 2017

mrmr1993 commented Aug 17, 2017 •

edited

Loading

philc commented Aug 17, 2017

mrmr1993 commented Aug 17, 2017

gdh1995 Aug 24, 2017

mrmr1993 Aug 24, 2017

gdh1995 Aug 25, 2017

mrmr1993 Aug 25, 2017

mrmr1993 commented Aug 27, 2017

smblott-github commented Aug 29, 2017

gdh1995 commented Aug 29, 2017

mrmr1993 commented Aug 30, 2017

ivanruvalcaba commented Sep 6, 2017

smblott-github commented Sep 7, 2017

gdh1995 commented Sep 7, 2017

mrmr1993 commented Sep 7, 2017

mrmr1993 commented Sep 7, 2017

gdh1995 commented Sep 7, 2017 •

edited

Loading

ghost commented Sep 21, 2017

smblott-github Sep 21, 2017

mrmr1993 Sep 21, 2017

mrmr1993 commented Sep 21, 2017

Firefox: Simulate default action for clicking links with link hints #2602

Firefox: Simulate default action for clicking links with link hints #2602

Conversation

mrmr1993 commented Aug 15, 2017

philc commented Aug 17, 2017

mrmr1993 commented Aug 17, 2017 • edited Loading

philc commented Aug 17, 2017

mrmr1993 commented Aug 17, 2017

gdh1995 Aug 24, 2017

Choose a reason for hiding this comment

mrmr1993 Aug 24, 2017

Choose a reason for hiding this comment

gdh1995 Aug 25, 2017

Choose a reason for hiding this comment

mrmr1993 Aug 25, 2017

Choose a reason for hiding this comment

mrmr1993 commented Aug 27, 2017

smblott-github commented Aug 29, 2017

gdh1995 commented Aug 29, 2017

mrmr1993 commented Aug 30, 2017

ivanruvalcaba commented Sep 6, 2017

smblott-github commented Sep 7, 2017

gdh1995 commented Sep 7, 2017

mrmr1993 commented Sep 7, 2017

mrmr1993 commented Sep 7, 2017

gdh1995 commented Sep 7, 2017 • edited Loading

ghost commented Sep 21, 2017

smblott-github Sep 21, 2017

Choose a reason for hiding this comment

mrmr1993 Sep 21, 2017

Choose a reason for hiding this comment

mrmr1993 commented Sep 21, 2017

mrmr1993 commented Aug 17, 2017 •

edited

Loading

gdh1995 commented Sep 7, 2017 •

edited

Loading