1376394 - Allow WebExtensions to read preference browser.tabs.warnOnCloseOtherTabs

Status: NEW

(WebExtensions :: General, enhancement, P3)

Description

[Geoffrey De Belie (Smile4ever)]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170626030209

Steps to reproduce:

Useful for vertical tab solutions.

Comment 1

[Bob Silverberg [:bsilverberg]]

Could you be a bit more detailed about the use case(s) for this?

Comment 2

[Geoffrey De Belie (Smile4ever)]

I'll try to explain this a bit.

In order to warn the user about the imminent closing of multiple tabs, a preference browser.tabs.warnOnCloseOtherTabs exists in Firefox. The "Close Other Tabs" functionality in Tab Center Redux currently closes tabs without warning because it is not possible to read that same preference.

In order to provide consistency across the native Firefox tab bar and Tab Center Redux it would be useful to read this preference.

The related preference browser.tabs.warnOnClose might be useful to read as well.

Comment 3

[Caitlin Neiman [:caitmuenster]]

Hi Geoffrey, this has been added to the agenda for the August 8 WebExtensions APIs triage. Looking forward to seeing you there! 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1I-i1FQ38nwFHNl4hnzTctdsYHXOjJpeAgm1Bm3VoUUA/edit#

Comment 4

[Geoffrey De Belie (Smile4ever)]

Use case info is here: https://github.com/eoger/tabcenter-redux/issues/93

Comment 5

[Bob Silverberg [:bsilverberg]]

In the meeting we discussed that we would be in favour of exposing the pref if it's something that is currently exposed to users via about:preferences and it is believed that it is something we plan to maintain long-term. If those statements are false, then we do not think we should expose the pref, and a workaround for extension authors would be to implement their own setting.

Our first investigation into whether this is exposed yielded odd results. One person could see the preference in about:preferences in Nightly, but nobody else could, so some more investigation is needed to determine if this is going to be a user-exposed, long-lived pref or not. I will take that investigation on.

Comment 6

[Bob Silverberg [:bsilverberg]]

browser.tabs.warnOnClose is exposed via about:preferences, but in an odd manner. If its value has been changed from the default (which is `true`), then it does appear in about:preferences allowing one to turn it back on. If its value has not been changed (meaning it's still `true`) then it does not appear in about:preferences at all. I assume this is because we would prefer that the average user not turn it off, but if it's off we want them to have a way of turning it back on.

browser.tabs.warnOnCloseOtherTabs is not exposed via about:preferences at all.

browser.tabs.warnOnClose controls whether the browser prompts a user when they are about to close ALL of the tabs in a window. The dialog that appears allows the user to change the preference via a checkbox, which accounts for the odd about:preferences behaviour described above.

browser.tabs.warnOnCloseOtherTabs controls whether the browser prompts a user when they are about to close multiple, but not all, tabs. The dialog shown does not include a checkbox to alter the pref, so the only way for this pref to be different from the default is if a user changed it manually via about:config.

Based on what I said in comment 5, it sounds like we'd be okay providing access to browser.tabs.warnOnClose, but not to browser.tabs.warnOnCloseOtherTabs, although the latter is what this bug is asking for. I think the key is whether browser.tabs.warnOnCloseOtherTabs is a pref that is likely to stick around (and retain its meaning) for a long time or not.

Gijs, might you have an idea about that, and if not could you suggest someone who might?

Comment 7

[Shane Caraveo (:mixedpuppy)]

Attachment: with extra pref.png

My normal profile on 57.0a1 (2017-08-08) (64-bit)

Comment 8

[Shane Caraveo (:mixedpuppy)]

Attachment: without extra pref.png

using a build profile I've had around forever on 57.0a1 (2017-08-08) (64-bit).  I cant seem to get the extra pref by flipping any other prefs.

Comment 9

[:Gijs (he/him)]

(In reply to Bob Silverberg [:bsilverberg] from comment #6)
> browser.tabs.warnOnClose is exposed via about:preferences, but in an odd
> manner. If its value has been changed from the default (which is `true`),
> then it does appear in about:preferences allowing one to turn it back on. If
> its value has not been changed (meaning it's still `true`) then it does not
> appear in about:preferences at all. I assume this is because we would prefer
> that the average user not turn it off, but if it's off we want them to have
> a way of turning it back on.

https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content-new/main.js#216-223

> browser.tabs.warnOnCloseOtherTabs is not exposed via about:preferences at
> all.
> 
> browser.tabs.warnOnClose controls whether the browser prompts a user when
> they are about to close ALL of the tabs in a window. The dialog that appears
> allows the user to change the preference via a checkbox, which accounts for
> the odd about:preferences behaviour described above.
> 
> browser.tabs.warnOnCloseOtherTabs controls whether the browser prompts a
> user when they are about to close multiple, but not all, tabs. The dialog
> shown does not include a checkbox to alter the pref, so the only way for
> this pref to be different from the default is if a user changed it manually
> via about:config.
> 
> Based on what I said in comment 5, it sounds like we'd be okay providing
> access to browser.tabs.warnOnClose, but not to
> browser.tabs.warnOnCloseOtherTabs, although the latter is what this bug is
> asking for. I think the key is whether browser.tabs.warnOnCloseOtherTabs is
> a pref that is likely to stick around (and retain its meaning) for a long
> time or not.
> 
> Gijs, might you have an idea about that, and if not could you suggest
> someone who might?

Put on safety goggles and see bug 772319 and bug 277079.

I have no idea if we intend to keep the prefs as they are. It seems part of the issue may have been that the "close other tabs" entry was next to "Close tab", or something, but it no longer is, so perhaps we can remove the additional pref and make it just 1 pref. Alternatively, from what you're saying, we could add a checkbox to the dialog and then hide the pref in about:preferences if it's set to the default, like we do for the other pref. That would also fix bug 801072. Though, tbh, reading those bugs I think people are going to be pissed off no matter what we do. Dão, thoughts given you were involved in the history here and I wasn't?

Comment 10

[Dão Gottwald [:dao]]

I don't understand why this would be useful for vertical tab solutions. Extension APIs used for such a thing should end up calling gBrowser.removeAllTabsBut which takes care of the hidden pref without the caller needing to know about it.

Comment 11

[Geoffrey De Belie (Smile4ever)]

(In reply to Dão Gottwald [::dao] from comment #10)
> I don't understand why this would be useful for vertical tab solutions.
> Extension APIs used for such a thing should end up calling
> gBrowser.removeAllTabsBut which takes care of the hidden pref without the
> caller needing to know about it.

gBrowser isn't even a thing for WebExtensions.

Comment 12

[:Gijs (he/him)]

(In reply to Geoffrey De Belie (Smile4ever) from comment #11)
> (In reply to Dão Gottwald [::dao] from comment #10)
> > I don't understand why this would be useful for vertical tab solutions.
> > Extension APIs used for such a thing should end up calling
> > gBrowser.removeAllTabsBut which takes care of the hidden pref without the
> > caller needing to know about it.
> 
> gBrowser isn't even a thing for WebExtensions.

Dão's point is that the method in question should be exposed via a new WebExtensions tabs.removeAllTabsBut() API, or that tabs.remove([list of all tabs but one]) should be clever and call the right method on gBrowser, instead of whatever it's doing now. Either way, making add-ons reimplement checking the right prefs is a losing battle.

Comment 13

[Geoffrey De Belie (Smile4ever)]

(In reply to :Gijs from comment #12)
> (In reply to Geoffrey De Belie (Smile4ever) from comment #11)
> > (In reply to Dão Gottwald [::dao] from comment #10)
> > > I don't understand why this would be useful for vertical tab solutions.
> > > Extension APIs used for such a thing should end up calling
> > > gBrowser.removeAllTabsBut which takes care of the hidden pref without the
> > > caller needing to know about it.
> > 
> > gBrowser isn't even a thing for WebExtensions.
> 
> Dão's point is that the method in question should be exposed via a new
> WebExtensions tabs.removeAllTabsBut() API, or that tabs.remove([list of all
> tabs but one]) should be clever and call the right method on gBrowser,
> instead of whatever it's doing now. Either way, making add-ons reimplement
> checking the right prefs is a losing battle.

We have https://bugzilla.mozilla.org/show_bug.cgi?id=1388462 for that.

Comment 14

[Bob Silverberg [:bsilverberg]]

Thanks for the feedback Gijs and Dão. All of this is sounding like we should not expose the prefs to WebExtensions and that other ways of dealing with this (like bug 1388462) should be considered instead. I think we should dupe this to bug 1388462.

Shane, what do you think?

Comment 15

[Shane Caraveo (:mixedpuppy)]

I don't really like the specific suggestion in bug 1388462, but I'll comment there on how I'd like to see it.  Lets make this dependent on that.  If the suggestions there don't pan out, we may still need to consider this.

Comment 16

[Shane Caraveo (:mixedpuppy)]

Keeping this open in case bug 1388462 is denied