Remove all colon signs on sub-dialogs.

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Preferences
P1
normal
VERIFIED FIXED
a year ago
10 months ago

People

(Reporter: evanxd, Assigned: evanxd)

Tracking

(Blocks: 1 bug)

55 Branch
Firefox 57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox57 verified)

Details

(Whiteboard: [photon-preference])

MozReview Requests

()

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

Attachments

(2 attachments)

(Assignee)

Description

a year ago
Remove all colon signs on sub-dialogs.
Flags: qe-verify+
(Assignee)

Updated

a year ago
Blocks: 1377330
(Assignee)

Updated

a year ago
Whiteboard: [photon-preference]
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8892332 - Flags: review?(jaws)
(Assignee)

Comment 6

a year ago
Hi Jared,

Could you help review the patch?
The patch is all about changing strings because we would like to remove all colon signs on sub-dialogs to align what we did on category pans.

Thank you.
Hi Evan, I see that this has a target milestone of 57. Is that correct? Are you trying to get this landed after 56 merges to Beta?
Flags: needinfo?(evan)
(Assignee)

Comment 8

a year ago
Hi Jared,

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Hi Evan, I see that this has a target milestone of 57. Is that correct? 
Yes, the target milestone is correct because this is in the scope of visual refresh.

> Are you trying to get this landed after 56 merges to Beta?
Yes, We'll land it after 56 merges to beta.
Flags: needinfo?(evan)
flod, do we need to rev all these IDs to remove the colon? I don't want to create a lot of busy work if it's not necessary.
Flags: needinfo?(francesco.lodolo)
Sadly I think we need new IDs, because most language follow English's style for this, e.g. https://transvision.mozfr.org/string/?entity=browser/chrome/browser/preferences/colors.dtd:textColor.label&repo=central
Flags: needinfo?(francesco.lodolo)

Comment 11

a year ago
mozreview-review
Comment on attachment 8892332 [details]
Bug 1382135 - Remove all colon signs on sub-dialogs because we already removed them all on each category pane and we should make same thing for sub-dialogs.

https://reviewboard.mozilla.org/r/163296/#review170302

r=me with the following strings and related accesskeys updated.

::: browser/components/preferences/in-content-new/privacy.xul:327
(Diff revision 5)
>            </hbox>
>            <hbox id="acceptThirdPartyRow"
>                  class="indent"
>                  align="center">
>              <label id="acceptThirdPartyLabel" control="acceptThirdPartyMenu"
>                     accesskey="&acceptThirdParty.pre.accesskey;">&acceptThirdParty.pre.label;</label>

This string still needs to be updated:

<!ENTITY acceptThirdParty.pre.label "Accept third-party cookies:">

::: browser/components/preferences/in-content-new/privacy.xul:343
(Diff revision 5)
>            <hbox id="keepRow"
>                  class="indent"
>                  align="center">
>              <label id="keepUntil"
>                     control="keepCookiesUntil"
>                     accesskey="&keepUntil.accesskey;">&keepUntil.label;</label>

This string still needs to be updated:

<!ENTITY keepUntil.label "Keep until:">

::: browser/components/preferences/in-content-new/privacy.xul:774
(Diff revision 5)
>                accesskey="&offlineStorageNotifyExceptions.accesskey;"/>
>      </hbox>
>    </hbox>
>    <hbox>
>      <vbox flex="1">
>        <label id="offlineAppsListLabel">&offlineAppsList2.label;</label>

This label still needs to be updated:

<!ENTITY offlineAppsList2.label "The following websites are allowed to store data for offline use:">
Attachment #8892332 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 14

a year ago
Thank you for the review, Jared.

Let's land it after the try[1] is good.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bab97c413a5
(Assignee)

Comment 15

a year ago
The try is good. Let's land it.
Keywords: checkin-needed

Comment 16

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d02d9abe10e
Remove all colon signs on sub-dialogs because we already removed them all on each category pane and we should make same thing for sub-dialogs. r=jaws
Keywords: checkin-needed

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8d02d9abe10e
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED

Comment 18

a year ago
mozreview-review
Comment on attachment 8892332 [details]
Bug 1382135 - Remove all colon signs on sub-dialogs because we already removed them all on each category pane and we should make same thing for sub-dialogs.

https://reviewboard.mozilla.org/r/163296/#review171134

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:35
(Diff revision 7)
>  notificationspermissionstext5=The following websites have requested to send you notifications. You can specify which websites are allowed to send you notifications.
>  notificationspermissionstitle2=Settings - Notification Permissions
>  invalidURI=Please enter a valid hostname
>  invalidURITitle=Invalid Hostname Entered
>  savedLoginsExceptions_title=Exceptions - Saved Logins
> -savedLoginsExceptions_desc2=Logins for the following websites will not be saved:
> +savedLoginsExceptions_desc2=Logins for the following websites will not be saved

Missed the string ID in this one.

Comment 19

11 months ago
Hi,
I'm trying to verify this bug in Nightly 57 but I really can't figure out what exactly should be tested.
Could you please provide us with some details?
Thanks.
Flags: needinfo?(evan)

Updated

11 months ago
Depends on: 1391610
(Assignee)

Comment 20

11 months ago
Hi Hani,

Looks like we need to wait for Bug 1391610 landed to verify this bug.

Thank you.
Flags: needinfo?(evan)
(Assignee)

Comment 21

10 months ago
Created attachment 8906992 [details]
colon-signs-should-be-removed.png

(In reply to Hani Yacoub from comment #19)
> Hi,
> I'm trying to verify this bug in Nightly 57 but I really can't figure out
> what exactly should be tested.
> Could you please provide us with some details?
> Thanks.

We should remove all colon signs on all sub-dialogs. For example, the patch removed the colon signs on the screenshot of previous Colors sub-dialog. So could you please check all sub-dialogs and check that we've removed all colon signs. Thank you.
(Assignee)

Updated

10 months ago
Flags: needinfo?(hani.yacoub)
(Assignee)

Comment 22

10 months ago
needinfo Hani to ensure he could get message.

Comment 23

10 months ago
Yes, I get it. 
So could we verify this on nightly? Or we should wait for Bug 1391610 to land?

Thanks.
Flags: needinfo?(hani.yacoub) → needinfo?(evan)
(In reply to Hani Yacoub from comment #23)
> Yes, I get it. 
> So could we verify this on nightly? Or we should wait for Bug 1391610 to
> land?

That bug doesn't affect English, the string has already changed when this bug was fixed.
Flags: needinfo?(evan)

Comment 25

10 months ago
Build ID: 20170912013600
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.

There were a couple of colon signs displayed on sub-dialog so I logged this Bug 1399075.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.