Closed Bug 1447978 Opened 7 years ago Closed 6 years ago

First person used in history settings

Categories

(Firefox :: Settings UI, defect, P5)

Product:
Firefox
Component:
Settings UI
Version:
61 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: winson.wen1, Assigned: alexandersonone, Mentored)

References

Details

(Keywords: ux-consistency)

Attachments

(2 files, 1 obsolete file)

Capture.PNG
25.56 KB, image/png
Details
Bug 1447978 - Update strings to remove first person,
59 bytes, text/x-review-board-request
jaws
: review+
flod
: review+
sfoster
: feedback+
Details
Attached image Capture.PNG — 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180321220044

Steps to reproduce:

Open about:preferences
Click the Privacy & Security tab
Change history settings to use custom settings
Enable "Clear history when Nightly closes" and click Settings... next to it.


Actual results:

First person "Remember my browsing and download history" is used.
First person "When I quit Nightly, it should automatically clear all"



Expected results:

First person is not used, example "Remember browsing and download history"
and "When closed, Nightly should automatically clear all"
Severity: normal → trivial
Has Regression Range: --- → irrelevant
Has STR: --- → yes
Component: Untriaged → Preferences
OS: Unspecified → All
Hardware: Unspecified → All
Note, preferences strings are currently in the process of being migrated to use Fluent (bug 1446180), so this should probably wait until that settles.

Flagging :flod to get this on he and :gandalf's radar
Flags: needinfo?(francesco.lodolo)
Priority: -- → P5
I see bug 1446180 got reviewed. I'll mark this as blocked by that, so this one is free to work on when that lands.

In the meantime, I assume we still need new strings to be approved for this.
Depends on: 1446180
Flags: needinfo?(francesco.lodolo)
If you can agree on new strings, I can introduce them in my patch to bug 1446180 and save us a string invalidation again :)
Michelle, can you review the proposed new wording in comment #0?
Flags: needinfo?(mheubusch)
These are fine.  Thanks! NI'ing JSavory in case she has updates for the prefs UI based on prior discussions)
Flags: needinfo?(mheubusch) → needinfo?(jsavory)
I don't currently have any updates for the history section as of yet. There has been discussions about looking into this section of the prefs in the future to improve the UI, but no changes have been made yet.
Flags: needinfo?(jsavory)
I'd like to work on this bug.
I've assigned you. Please flag :jaws as your reviewer. You will need to increment the string names, e.g. 'history-remember-option' will become 'history-remember-option2', and find all the places that string is used and make the same change there.
Assignee: nobody → alexandersonone
Mentor: sfoster
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249116

Looks clean. With the suggested changes, you can push to review again. Add r?jaws to the end of your commit message to request review from Jared.

::: commit-message-aabfe:1
(Diff revision 1)
> +Fix bug 1447978

Commit message should take the form "Bug 1447978 - Update strings to remove first person."

::: browser/locales/en-US/chrome/browser/sanitize.dtd:16
(Diff revision 1)
>  
>  <!ENTITY sanitizeDialog2.title         "Clear Recent History">
>  <!-- LOCALIZATION NOTE (sanitizeDialog2.width): width of the Clear Recent History dialog -->
>  <!ENTITY sanitizeDialog2.width         "34em">
>  
> -<!ENTITY clearDataSettings3.label     "When I quit &brandShortName;, it should automatically clear all">
> +<!ENTITY clearDataSettings4.label     "When closed, &brandShortName; it should automatically clear all">

Should read "When closed, &brandShortName; should automatically clear all"
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249118

::: commit-message-aabfe:1
(Diff revision 1)
> +Fix bug 1447978

You should also add the reviewer, in form of r?jaws at the end of the commit.

```
Bug 1447978 - Update strings to remove first person, r?jaws
```

::: browser/locales/en-US/browser/preferences/preferences.ftl:699
(Diff revision 1)
>  
>  history-private-browsing-permanent =
>      .label = Always use private browsing mode
>      .accesskey = p
>  
> -history-remember-option =
> +history-remember-option2 =

Adding numbers to an existing string ID should be the last resort. I would suggest to use something like "history-remember-browser-option"

::: browser/locales/en-US/chrome/browser/sanitize.dtd:16
(Diff revision 1)
>  
>  <!ENTITY sanitizeDialog2.title         "Clear Recent History">
>  <!-- LOCALIZATION NOTE (sanitizeDialog2.width): width of the Clear Recent History dialog -->
>  <!ENTITY sanitizeDialog2.width         "34em">
>  
> -<!ENTITY clearDataSettings3.label     "When I quit &brandShortName;, it should automatically clear all">
> +<!ENTITY clearDataSettings4.label     "When closed, &brandShortName; it should automatically clear all">

Since this string is already numbered, no issue with just increasing the number.
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249118

> Adding numbers to an existing string ID should be the last resort. I would suggest to use something like "history-remember-browser-option"

ok thanks for that, flod. Take note Alex ^.
Certainly! Thank you for the knowledge.
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

Replaced by attachment 8975215 [details]
Attachment #8974871 - Attachment is obsolete: true
Oh I see what happened - I was confused there for a minute by the 2 attachments. Alex, can you flatten the 2 commits into 1 and re-push? I would suggest using hg fold, which requires the histedit extension. If you don't already have that, see https://www.mercurial-scm.org/wiki/HisteditExtension - let me know if I can help.
Flags: needinfo?(alexandersonone)
Attachment #8975215 - Attachment is obsolete: true
Attachment #8975215 - Flags: review?(jaws)
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249466

::: browser/locales/en-US/chrome/browser/sanitize.dtd:16
(Diff revision 3)
>  
>  <!ENTITY sanitizeDialog2.title         "Clear Recent History">
>  <!-- LOCALIZATION NOTE (sanitizeDialog2.width): width of the Clear Recent History dialog -->
>  <!ENTITY sanitizeDialog2.width         "34em">
>  
> -<!ENTITY clearDataSettings3.label     "When I quit &brandShortName;, it should automatically clear all">
> +<!ENTITY clearDataSettings4.label     "When closed, &brandShortName; it should automatically clear all">

If you substitute "&brandShortName;" for "Firefox" this will read "When closed, Firefox it should automatically clear all" which doesnt make sense. We want "When closed, Firefox should automatically clear all" so you just need to delete the "it "
Ahh, How did I miss this! It's fixed now though. Thanks :sfoster!
Flags: needinfo?(alexandersonone)
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249492

l10n looks good, giving it an official r+ since it includes changes to FTL files.
Attachment #8974871 - Flags: review+
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

Looks good, thanks. We just need an r+ from Jared or another peer now.
Attachment #8974871 - Flags: feedback+
Comment on attachment 8974871 [details]
Bug 1447978 - Update strings to remove first person,

https://reviewboard.mozilla.org/r/243262/#review249558
Attachment #8974871 - Flags: review?(jaws) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/091cdd176a11
Update strings to remove first person, r=flod,jaws
https://hg.mozilla.org/mozilla-central/rev/091cdd176a11
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
I have reproduced this bug with Nightly 61.0a1 (2018-04-15) on Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20180624100132
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
QA Whiteboard: [bugday-20180620]
QA Whiteboard: [bugday-20180620] → [bugday-20180620] [good first verify]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: