1455296 - The “Confirm close” dialog does not inform you about how many hidden tabs you have

Status: NEW

(Firefox :: Tabbed Browser, defect, P2)

Description

[CosminB]

Attachment: ConfirmClose.gif

[Affected versions]:
- Firefox 61.0a1 (20180419100148)

[Affected platforms]:
- Win 7 64-bit
- Mac OS 10.13.2

[Steps to reproduce]:
1.Flip “extensions.webextensions.tabhide.enabled” to true.
2.Open multiple tabs.
3.Hide some tabs.
4.Close the browser.
5.Observe the “Confirm close” dialog.

[Expected results]:
- The number of the hidden tabs is mentioned as presented in the https://mozilla.invisionapp.com/share/82EIATQAF#/screens/280879351 

[Actual results]:
- The number of the hidden tabs it is not mentioned.

Comment 1

[Mark Striemer [:mstriemer]]

Attachment: Bug 1455296 - Include hidden tab count in the confirm close dialog

Review commit: https://reviewboard.mozilla.org/r/242176/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242176/

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8974835 [details]
Bug 1455296 - Include hidden tab count in the confirm close dialog

https://reviewboard.mozilla.org/r/242176/#review249198

::: browser/base/content/tabbrowser.js:2540
(Diff revision 1)
>          if (!aTab)
>            throw new Error("Required argument missing: aTab");
>  
> -        tabsToClose = this.getTabsToTheEndFrom(aTab).length;
> +        let tabs = this.getTabsToTheEndFrom(aTab);
> +        tabsToClose = tabs.length;
> +        hiddenTabsToClose = getHiddenTabCount(tabs);

I don't think we want to include hidden tabs in the TO_END case.

Comment 3

[Mark Striemer [:mstriemer]]

Comment on attachment 8974835 [details]
Bug 1455296 - Include hidden tab count in the confirm close dialog

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242176/diff/1-2/

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8974835 [details]
Bug 1455296 - Include hidden tab count in the confirm close dialog

https://reviewboard.mozilla.org/r/242176/#review252140

::: browser/locales/en-US/chrome/browser/tabbrowser.properties:25
(Diff revision 2)
> +# tabs.closeWarningWithHidden which would be like "You are about to close 5 tabs...".
> +tabs.tabName=1 tab;#1 tabs
> +# LOCALIZATION NOTE (tabs.closeWarningWithHidden):
> +# %1$S and %2$s will be strings like "5 tabs". These are generated with the
> +# tabs.tabName string.
> +tabs.closeWarningWithHidden=You are about to close %1$S, including %2$S hidden by an extension. Are you sure you want to continue?

I'm not sure that slicing the string like this and then injecting tabs.tabName into tabs.closeWarningWithHidden will work for all locales.

Comment 5

[Francesco Lodolo [:flod]]

(In reply to Dão Gottwald [::dao] from comment #4)
> I'm not sure that slicing the string like this and then injecting
> tabs.tabName into tabs.closeWarningWithHidden will work for all locales.

I agree, that's non a safe approach.

To make things more complicated, plural support in properties is limited, and only allows plurals based on one value (here you need two).

Note that you need to add the exact comment before a string with plural forms, and use #1 for the variable used to determine the plural form
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Use_proper_plural_forms

If you want to keep the current English sentence, you could use a third string to glue two separate plural strings together.

# LOCALIZATION NOTE (tabs.closeWarningWithHidden):
# %1$S will be replaced by tabs.closeWarningOpenTabs
# %2$S will be replaced by tabs.closeWarningHiddenTabs
tabs.closeWarningWithHidden = %1$S, %2$S. Are you sure you want to continue?

# LOCALIZATION NOTE (tabs.closeWarningOpenTabs): Semi-colon list of plural forms.
# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
# #1 is the number of open tabs
tabs.closeWarningOpenTabs=You are about to close 1 tab;You are about to close #1 tabs

# LOCALIZATION NOTE (tabs.closeWarningHiddenTabs): Semi-colon list of plural forms.
# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
# #1 is the number of hidden tabs
tabs.closeWarningHiddenTabs=1 tab hidden by an extension;#1 tabs hidden by an extension

Would Fluent be an option here? It supports multiple plurals

tabs-close-warning-with-hidden =
    You are about to close { $opentabs ->
        [one] 1 tab
       *[other] { $opentabs } tabs
    }, { $hiddentabs ->
       *[one] 1 tab hidden by an extension
        [other] { $hiddentabs } tabs hidden by an extension
    }. Are you sure you want to continue?

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8974835 [details]
Bug 1455296 - Include hidden tab count in the confirm close dialog

https://reviewboard.mozilla.org/r/242176/#review252490

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 121 votes.
:mstriemer, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 8

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.