1697830 - Rename the "Auto hide" checkbox in the download button's panel in customize mode for consistency with the context menu

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, enhancement, P3)

Description

[Oriol Brufau [:Oriol]]

In the context menu entry for the downloads button, bug 1690575 renamed "Auto-Hide" to "Hide Button When Empty".

However, in customize mode, left-clicking the download button still shows "Auto-hide".
https://searchfox.org/mozilla-central/rev/ca22dc040c3a6f9257efdaedec848bb48334f8bd/browser/locales/en-US/chrome/browser/browser.dtd#169

It would be good to make it consistent with the context menu item.

Comment 1

[:Gijs (he/him)]

Dhanesh, would you like to work on this? You'd want to add a new fluent message in customizeMode.ftl; you can use https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/locales/en-US/browser/customizeMode.ftl#42-43 as an example, but you probably want to make the identifier something like customize-mode-downloads-button-autohide, still with a label attribute that has the "Hide button when empty" string value (note: sentence case, not title case like in comment #0).

You'd want to then change the code at https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/components/customizableui/content/panelUI.inc.xhtml#215-217 to remove the "label" attribute and use data-l10n-id to point to the fluent identifier that you added in customizeMode.ftl.

Comment 2

[Sarah Ukoha]

I think the exact Wordings have been added already as a fluent message in https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/toolbarContextMenu.ftl#48
If this has not been assigned, can I give it a go? Would appreciate the opportunity...

Comment 3

[:Gijs (he/him)]

(In reply to Sarah Ukoha from comment #2)

I think the exact Wordings have been added already as a fluent message in https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/toolbarContextMenu.ftl#48

The capitalization is different and we don't want an accesskey here, so no, we can't reuse it.

If this has not been assigned, can I give it a go? Would appreciate the opportunity...

Dhanesh asked to be assigned a different bug for the downloads project that wasn't actionable, so I passed him this one instead, so unfortunately no, I think it would be fairer for Dhanesh to have a crack at this one first.

Comment 4

[Sarah Ukoha]

(In reply to :Gijs (he/him) from comment #3)

(In reply to Sarah Ukoha from comment #2)

I think the exact Wordings have been added already as a fluent message in https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/toolbarContextMenu.ftl#48

The capitalization is different and we don't want an accesskey here, so no, we can't reuse it.

If this has not been assigned, can I give it a go? Would appreciate the opportunity...

Dhanesh asked to be assigned a different bug for the downloads project that wasn't actionable, so I passed him this one instead, so unfortunately no, I think it would be fairer for Dhanesh to have a crack at this one first.

Oh! It's fine. Thanks!! Will keep looking

Comment 5

[Dhanesh]

yea gijs. I really want to work on this issue and Thanks for recommending this to me without demand.

Comment 6

[Dhanesh]

(In reply to :Gijs (he/him) from comment #1)

Dhanesh, would you like to work on this? You'd want to add a new fluent message in customizeMode.ftl; you can use https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/locales/en-US/browser/customizeMode.ftl#42-43 as an example, but you probably want to make the identifier something like customize-mode-downloads-button-autohide, still with a label attribute that has the "Hide button when empty" string value (note: sentence case, not title case like in comment #0).

You'd want to then change the code at https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/components/customizableui/content/panelUI.inc.xhtml#215-217 to remove the "label" attribute and use data-l10n-id to point to the fluent identifier that you added in customizeMode.ftl.

@gijs you have said me to add the label string in sentence case but in customizeMode.ftl , some label values done in Title case format.

customize-mode-uidensity-menu-compact-unsupported =
.label = Compact (not supported)
.accesskey = C
.tooltiptext = Compact (not supported)
customize-mode-lwthemes-menu-get-more =
.label = Get More Themes
.accesskey = G
customize-mode-undo-cmd =
.label = Undo
customize-mode-lwthemes-my-themes =
.value = My Themes
customize-mode-touchbar-cmd =
.label = Customize Touch Bar…
customize-mode-downloads-button-autohide =
.label = Hide button when empty

see.I have added the line but the line before has a label attribute that has the value in Title format. So is it okay to proceed with the sentence case?

Comment 7

[Dhanesh]

ive replaced the label attribute in panel.xhtml file

<checkbox id="downloads-button-autohide-checkbox"
data-l10n-id="customize-mode-downloads-button-autohide" checked="true"
oncommand="gCustomizeMode.onDownloadsAutoHideChange(event)"/>

is it right? gijs

Comment 8

[:Gijs (he/him)]

(In reply to Dhanesh from comment #7)

ive replaced the label attribute in panel.xhtml file

<checkbox id="downloads-button-autohide-checkbox"
data-l10n-id="customize-mode-downloads-button-autohide" checked="true"
oncommand="gCustomizeMode.onDownloadsAutoHideChange(event)"/>

is it right? gijs

Yes, this seems correct.

(In reply to Dhanesh from comment #6)

(In reply to :Gijs (he/him) from comment #1)

Dhanesh, would you like to work on this? You'd want to add a new fluent message in customizeMode.ftl; you can use https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/locales/en-US/browser/customizeMode.ftl#42-43 as an example, but you probably want to make the identifier something like customize-mode-downloads-button-autohide, still with a label attribute that has the "Hide button when empty" string value (note: sentence case, not title case like in comment #0).

You'd want to then change the code at https://searchfox.org/mozilla-central/rev/3ae6f9e4444f3fbf61034317540d36f621117600/browser/components/customizableui/content/panelUI.inc.xhtml#215-217 to remove the "label" attribute and use data-l10n-id to point to the fluent identifier that you added in customizeMode.ftl.

@gijs you have said me to add the label string in sentence case but in customizeMode.ftl , some label values done in Title case format.
<snip>
customize-mode-touchbar-cmd =
.label = Customize Touch Bar…
customize-mode-downloads-button-autohide =
.label = Hide button when empty

see.I have added the line but the line before has a label attribute that has the value in Title format. So is it okay to proceed with the sentence case?

Yes, "Touch Bar" is a name, and the string we're adding is more of a sentence than a single concept anyway (it has a verb and subject, even!), and we're slowly but surely switching things to sentence case, so I think this is fine.

Comment 9

[Dhanesh]

Attachment: Bug 1697830 Make a checkbox label in the download button's customize mode tooltip consistent with the context menu equivalent.

Depends on D111410

Comment 10

[Dhanesh]

yea gijs. Ive done ur requested changes.
Thanks for mentoring me :)
Could you assign me another bug to work on ?

Thanks,
Dhanesh

Comment 11

[Dhanesh]

gijs . i would to like to work on this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1656573 . Could you assign me this ?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06c86943b120
Make the checkbox label in the download button's customize mode tooltip consistent with the context menu equivalent. r=Gijs,fluent-reviewers

Comment 13

[:Gijs (he/him)]

Attachment: Bug 1697830 - follow-up: remove now-unused DTD string, r?flod

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b032bddd5d86
follow-up: remove now-unused DTD string, r=flod

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/06c86943b120
https://hg.mozilla.org/mozilla-central/rev/b032bddd5d86

Comment 16

[Bogdan Maris, Desktop Test Engineering]

Verified that Auto hide is now changed to Hide Button When Empty when Download button is located inside tools palette or toolbar. Verified using Firefox 89.0b3 across platforms (Windows 10, Ubuntu 18.04 and macOS 11.2)