Closed
Bug 1079430
Opened 10 years ago
Closed 10 years ago
Land the new Loop strings for Firefox 35
Categories
(Hello (Loop) :: Client, defect)
Hello (Loop)
Client
Tracking
(firefox35 fixed)
Tracking | Status | |
---|---|---|
firefox35 | --- | fixed |
People
(Reporter: jaws, Assigned: jaws)
References
()
Details
Attachments
(1 file, 1 obsolete file)
20.14 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Comment 1•10 years ago
|
||
Just a reminder: The first panel for the FTE tour (shown here: http://cl.ly/image/433q0j2K0o1x) is in-product, so the strings for that ("Firefox Hello -- Join the conversation" and "Get Started") need to be included in this bug. These are on lines 2 and 5 of this spreadsheet: https://docs.google.com/a/mozilla.com/spreadsheets/d/1dRhHrnjWIM5cWoKKXs71DHJwZ_nIFeTxwg8zyoFKgiA/edit#gid=0
Assignee | ||
Comment 2•10 years ago
|
||
> +pref("loop.learnMoreUrl", ""); I left this pref blank for now until we have the link to use for it. I tested this and the email body still works it just shows blank where the URL should be. If you are OK with this I will file a follow-up to get this URL added, but it shouldn't block uplift because it won't break string freeze. > - // XXX Bug 1013989 will provide a label for the button I deleted this line since it doesn't apply for mozilla-central anymore, but bug 1013989 should stay open since it is still needed for Firefox 34 (this bug is only targeting Firefox 35). > +++ b/browser/locales/en-US/chrome/browser/loop/loop.properties > +clientShortname2=Firefox Hello This string didn't exist in loop.properties before, but I went with using clientShortname2 instead of clientShortname so it would be consistent with loop.en-US.properties. remote: You can view the progress of your build at the following URL: remote: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=765ea07ec66e remote: Alternatively, view them on TBPL (soon to be deprecated): remote: https://tbpl.mozilla.org/?tree=Try&rev=765ea07ec66e
Attachment #8501339 -
Flags: review?(MattN+bmo)
Comment 3•10 years ago
|
||
Comment on attachment 8501339 [details] [diff] [review] Patch Review of attachment 8501339 [details] [diff] [review]: ----------------------------------------------------------------- I didn't have time to review this today so someone else can feel free to review before me. ::: browser/components/loop/MozLoopService.jsm @@ +1235,5 @@ > */ > getStrings: function(key) { > var stringData = MozLoopServiceInternal.localizedStrings; > if (!(key in stringData)) { > + Cu.reportError('No string found for key: ' + key); While you're touching this you could change it to log.error for logging consistency within the file.
Attachment #8501339 -
Flags: review?(mdeboer)
Comment 4•10 years ago
|
||
Comment on attachment 8501339 [details] [diff] [review] Patch Review of attachment 8501339 [details] [diff] [review]: ----------------------------------------------------------------- Some of the comments for loop.properties also apply to loop.en-US.properties. ::: browser/components/loop/content/js/panel.jsx @@ +385,5 @@ > "callUrl": !this.state.pending > }); > return ( > <div className="generate-url"> > + <header>{__("share_link_header_text2")}</header> This shouldn't change. This is the call-url UI, that isn't changing for rooms. Additionally the new string here "Invite someone to join you", is for the conversation window. @@ +393,5 @@ > <ButtonGroup additionalClass="url-actions"> > <Button additionalClass="button-email" > disabled={!this.state.callUrl} > onClick={this.handleEmailButtonClick} > + caption={mozL10n.get("share_button_label")} /> Likewise, this is part of the existing UI, so I don't think we should change it, just add the new string. @@ +398,5 @@ > <Button additionalClass="button-copy" > disabled={!this.state.callUrl} > onClick={this.handleCopyButtonClick} > caption={this.state.copied ? mozL10n.get("copied_url_button") : > + mozL10n.get("copy_url_button_label")} /> Ditto re not changing existing UI. ::: browser/locales/en-US/chrome/browser/loop/loop.properties @@ +3,5 @@ > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > # Panel Strings > > +clientShortname2=Firefox Hello I'm not sure if this should actually be localizable. It should definitely get a L10n note saying this is a brand name and don't localize, if we leave it in here. @@ +251,3 @@ > ## from legal_text_tos and legal_text_privacy. clientShortname will be replaced > ## by the brand name, or fall back to client_shortname_fallback > legal_text_and_links3=By using {{clientShortname}} you agree to the {{terms_of_use}} \ Can you change the l10n note for this, to go with the change in panel.jsx, and remove the client_shortname_fallback? @@ +288,5 @@ > + > +help.label=Help > +tour.label=Tour > + > +rooms.default_room_name_template=Conversation {{conversationLabel}} We should add an L10n note here explaining that this is replaced by a number. I'm vaguely wondering if this needs to be a plural form, but I'm not sure it does. @@ +293,5 @@ > +rooms.leave_button.label=Leave > +rooms.list.copy_url.tooltip=Copy Link > +rooms.list.delete.tooltip=Delete Conversation > +rooms.list.deleteConfirmation.label=Are you sure? > +rooms.list.title=Current Conversations Bug 1074672 is adding a better version of this. @@ +296,5 @@ > +rooms.list.deleteConfirmation.label=Are you sure? > +rooms.list.title=Current Conversations > +rooms.name_this_room.label=Name this conversation > +rooms.new_room_button.label=Start a Conversation > +rooms.no_rooms.label=No Current Conversations Bug 1074672 is adding this.
Attachment #8501339 -
Flags: review-
Comment 5•10 years ago
|
||
Comment on attachment 8501339 [details] [diff] [review] Patch Review of attachment 8501339 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/standalone/content/l10n/loop.en-US.properties @@ +92,5 @@ > +rooms.list.delete.tooltip=Delete Conversation > +rooms.list.deleteConfirmation.label=Are you sure? > +rooms.list.title=Current Conversations > +rooms.name_this_room.label=Name this conversation > +rooms.new_room_button.label=Start a Conversation I think someone needs to address this inconsistent capitalizing of 'Conversation'. ::: browser/locales/en-US/chrome/browser/loop/loop.properties @@ +3,5 @@ > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > # Panel Strings > > +clientShortname2=Firefox Hello +1, I'd be for handling this like we handle the 'Firefox' or 'Nightly' brand name(s)
Attachment #8501339 -
Flags: review?(mdeboer)
Attachment #8501339 -
Flags: review?(MattN+bmo)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8501339 -
Attachment is obsolete: true
Attachment #8502215 -
Flags: review?(standard8)
Comment 7•10 years ago
|
||
Comment on attachment 8502215 [details] [diff] [review] Patch Review of attachment 8502215 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. r=Standard8
Attachment #8502215 -
Flags: review?(standard8) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3e9de16c4c17
Flags: in-testsuite+
Flags: firefox-backlog+
Whiteboard: [fixed in fx-team]
https://hg.mozilla.org/mozilla-central/rev/3e9de16c4c17
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in fx-team]
Target Milestone: --- → mozilla35
status-firefox35:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•