Why overriding browser.link.open_newwindow?

saadtazi / firefox-profile-js

Firefox Profile creation using nodejs and CLI

MIT License

60 stars 30 forks source link

Why overriding browser.link.open_newwindow? #104

Open emilio opened 4 years ago

emilio commented 4 years ago

https://github.com/saadtazi/firefox-profile-js/blob/397764d1d6868cecabbe4a29a144d1b4f1ef7e49/lib/firefox_profile.js#L32

Changes the behavior of target="_blank" links. This is problematic for two reasons:

It doesn't work on Android (see https://github.com/mozilla/web-ext/issues/1592 for example)
Dependent libraries have very confusing behavior as a result, so that when I'm testing web-extensions my links misbehave. I thought at first it was a browser bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1598947).

If there's a good reason to change default behavior like that, it'd be good to state why in a comment at least? :)

saadtazi commented 4 years ago

Hi @emilio ,

The default preference were ported from the old python selenium code, which used to be located here (when I created this extension) but has been moved to here. The default prefs are now defined ~~here~~ here (using this build file).

So it seems like the default behaviour is still to use 'browser.link.open_newwindow': '2'. But I might have missed something.

saadtazi commented 4 years ago

I checked my firefox install, it is set to 3 by default...

Note for self: here is an explanation of the different possible values for browser.link.open_newwindow: http://kb.mozillazine.org/Browser.link.open_newwindow .

This link states that the default value is 3 for firefox 2.0 (??? 👀 ???) and newer...

Summary: So this library seems to use the default value used by the default selenium libraries, but I don't understand why the selenium libraries still default to browser.link.open_newwindow = 2.

Any thoughts?

emilio commented 4 years ago

Not sure, the default value in Gecko is defined here: https://searchfox.org/mozilla-central/rev/a78233c11a6baf2c308fbed17eb16c6e57b6a2ac/browser/app/profile/firefox.js#431

But not sure why other libraries would change it.

saadtazi commented 4 years ago

I wouldn't mind setting it to 3, which will require increasing the major version of this library. Feel free to open a PR if you have time. If you don't have time but still think web-ext could benefit from it, let me know, I'll do it.

emilio commented 4 years ago

I sent https://github.com/mozilla/web-ext/pull/1766 right before opening this issue for web-ext.

I don't know about other users of this library.