Closed Bug 1338036 Opened 7 years ago Closed 7 years ago

Switch button label from "Remove All" to "Remove All Shown" when filtering by keyword in about:preferences

Categories

(Firefox :: Settings UI, defect)

Product:
Firefox
Component:
Settings UI
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: Fischer, Assigned: Fischer)

References

Details

(Whiteboard: [storage-v1])

Attachments

(1 file)

Bug 1338036 - Switch button label from "Remove All" to "Remove All Shown" when filtering by keyword in about:preferences,
59 bytes, text/x-review-board-request
jaws
: review+
Details 
When only partial items on the list are shown as a result of keyword search, "Remove All Shown" should be displayed as button label. "Remove All" should displayed when no keyword search and all items are shown. This is to make user clear that the functionality of the button is to remove all listed items only.
The dialogs in about:preferences to do are
- Saved-Logins dialog
- Cookies dialog
- Site Data Settings dialog
Assignee: nobody → fliu
Depends on: 312380
No longer depends on: 1312372, 1312380
No longer depends on: 312380
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #1)
> I think you mixed up bug 1312380 with bug 312380
Thanks.
Depends on: 1312380
Comment on attachment 8835324 [details]
Bug 1338036 - Switch button label from "Remove All" to "Remove All Shown" when filtering by keyword in about:preferences,

Jaws,
This is a follow-up bug of the bug 1312380 [1], thanks
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1312380#c7
Attachment #8835324 - Flags: review?(jaws)
Comment on attachment 8835324 [details]
Bug 1338036 - Switch button label from "Remove All" to "Remove All Shown" when filtering by keyword in about:preferences,

https://reviewboard.mozilla.org/r/110982/#review112600

r=me, but it is important that you make the following changes before checking this in. I won't ask you to request review on this for just those changes, but if you need to make other changes then please request review again.

::: browser/components/preferences/cookies.js:876
(Diff revision 1)
>  
>    _updateRemoveAllButton: function gCookiesWindow__updateRemoveAllButton() {
> -    document.getElementById("removeAllCookies").disabled = this._view._rowCount == 0;
> +    let removeAllCookies = document.getElementById("removeAllCookies");
> +    let label = this._view._filtered ?
> +      this._bundle.getString("removeAllShownCookies") : this._bundle.getString("removeAllCookies");
> +    removeAllCookies.disabled = this._view._rowCount == 0;

nit, please move the 'removeAllCookies.disabled' line up so it is right after the removeAllCookies variable is defined.

::: browser/locales/en-US/chrome/browser/preferences/cookies.dtd:9
(Diff revision 1)
> -<!ENTITY     cookiedomain.label             "Site"> 
> +<!ENTITY     cookiedomain.label             "Site">
>  <!-- LOCALIZATION NOTE (button.removeSelectedCookies.accesskey):
> -  The label associated with this accesskey can be found in 
> +  The label associated with this accesskey can be found in
>    preferences.properties as removeSelectedCookies.
>  -->
>  <!ENTITY     button.removeSelectedCookies.accesskey "R">
> -<!ENTITY     button.removeAllCookies.label "Remove All">
> +<!-- LOCALIZATION NOTE (button.removeSelectedCookies.accesskey):
> +  The label associated with this accesskey can be found in
> +  preferences.properties as removeAllCookies/removeAllShownCooikes.
> +-->

These accesskeys should be moved to the .properties file. It is best to always keep the label and accesskey next to each other.

::: browser/locales/en-US/chrome/browser/preferences/siteDataSettings.dtd:14
(Diff revision 1)
> -<!ENTITY     removeAll.label               "Remove All">
> +<!-- LOCALIZATION NOTE (removeAll.accesskey):
> +  The label associated with this accesskey can be found in
> +  preferences.properties as removeAll.label/removeAllShown.label
> +-->

This accesskey should be moved to the properties file. The label should always be next to the accesskey.
Attachment #8835324 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Comment on attachment 8835324 [details]
> Bug 1338036 - Switch button label from "Remove All" to "Remove All Shown"
> when filtering by keyword in about:preferences
> 
> https://reviewboard.mozilla.org/r/110982/#review112600
> 
> r=me, but it is important that you make the following changes before
> checking this in. I won't ask you to request review on this for just those
> changes, but if you need to make other changes then please request review
> again.
> 
Thanks, will update accordingly and request review again if more other changes.
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc4ac23cf72b
Switch button label from "Remove All" to "Remove All Shown" when filtering by keyword in about:preferences, r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fc4ac23cf72b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Depends on: 1338757
I have successfully reproduced this bug in nightly 2017-02-08 in Windows7,64Bit .

This bug is fix is verified in latest Beta.

BuildID:20170511130324
User Agent:Mozilla/5.0 (Windows NT 10.0; rv:54.0) Gecko/20100101 Firefox/54.0

[testday-20170512]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: