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

RESOLVED FIXED in Firefox 54

Status

()

Firefox
Preferences
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: Fischer, Assigned: Fischer)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [storage-v1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
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)

Updated

7 months ago
Assignee: nobody → fliu
Depends on: 312380
No longer depends on: 1312372, 1312380
No longer depends on: 312380
I think you mixed up bug 1312380 with bug 312380
(Assignee)

Comment 2

7 months ago
(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 hidden (mozreview-request)
(Assignee)

Comment 4

7 months ago
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 5

6 months ago
mozreview-review
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+
(Assignee)

Comment 6

6 months ago
(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.
Comment hidden (mozreview-request)
(Assignee)

Comment 8

6 months ago
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e32f9eeff00a0194a21481412318688bda8bc02c
Keywords: checkin-needed

Comment 9

6 months ago
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

Comment 10

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fc4ac23cf72b
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Depends on: 1338757

Comment 11

3 months ago
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.