TB shows instructions with [File] - [Offline] - [Synchronize] instead of [Download/Sync Now]

RESOLVED FIXED in Thunderbird 40.0

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pollti, Assigned: pollti)

Tracking

Trunk
Thunderbird 40.0

Thunderbird Tracking Flags

(thunderbird40 fixed)

Details

Attachments

(1 attachment, 7 obsolete attachments)

Posted patch patch (obsolete) — Splinter Review
When TB is offline and a message body could not be downloaded, TB shows the following message:
>The body of this message has not been downloaded from
>the server for reading offline. To read this message,
>you must reconnect to the network, choose Offline from
>the File menu and then uncheck Work Offline.
>In the future, you can select which messages or folders to read offline. To do
>this, choose Offline from the file menu and then select Synchronize. You can
>adjust the Disk Space preference to prevent the downloading of large messages.
"Synchronize" should be replaced by "Download/Sync Now…"
Attachment #8582517 - Flags: review?(bwinton)
Comment on attachment 8582517 [details] [diff] [review]
patch

Review of attachment 8582517 [details] [diff] [review]:
-----------------------------------------------------------------

Other than the nit below, I like it.
Adding Neil for the review of the Suite code.

::: mail/locales/en-US/chrome/messenger/messenger.properties
@@ +306,5 @@
>  the server for reading offline. To read this message, \
>  you must reconnect to the network, choose Offline from \
>  the File menu and then uncheck Work Offline. \
>  In the future, you can select which messages or folders to read offline. To do \
> +this, choose Offline from the file menu and then select Download/Sync Now…. \

I would remove the … from this, since it doesn't add any benefit, and looks confusing.
Attachment #8582517 - Flags: review?(neil)
Attachment #8582517 - Flags: review?(bwinton)
Attachment #8582517 - Flags: review+
Comment on attachment 8582517 [details] [diff] [review]
patch

Whoa, regression from bug 71125 ;-)

Unfortunately due to our sucky l10n process we need to change the string key (here and in the C++) so that localisers notice the problem.
Attachment #8582517 - Flags: review?(neil) → review-
Posted patch patch #2 (obsolete) — Splinter Review
Thanks for your feedback! This removes the "…".
Neil, could you explain, what I have to do, please? (I'm sorry - I'm not very familiar with TB l10n update processes.)
Attachment #8582517 - Attachment is obsolete: true
Flags: needinfo?(neil)
Summary: TB shows instructions with [File] - [Offline] - [Synchronize] instead of [Download/Sync Now…] → TB shows instructions with [File] - [Offline] - [Synchronize] instead of [Download/Sync Now]
Comment on attachment 8583165 [details] [diff] [review]
patch #2

> nocachedbodybody=The body of this message has not been downloaded from \
The name "nocachedbodybody" identifies this message. Unfortunately if you change the message without changing the name, then localisers will not realise that the message has changed, as the l10n dashboard will only display differences in names. You therefore need to change the name, not only in these two files, but also in the C++ file that also references this message.
Flags: needinfo?(neil)
Posted patch patch, v3 (obsolete) — Splinter Review
Thanks! This should update the string's name in all the files.
Attachment #8583165 - Attachment is obsolete: true
Attachment #8584977 - Flags: review?(neil)
Attachment #8584977 - Flags: review?(bwinton)
Comment on attachment 8584977 [details] [diff] [review]
patch, v3

> nocachedbodybodyupdated=The body of this message has not been downloaded from \
Somehow this isn't showing up as a change in your patch?
Also, when changing strings, we normally use a numeric suffix, in case the string ever needs to be changed again (i.e. the suffix is 2, 3, etc. for each subsequent change).
I don't see the reason for the not working changes at the moment. I'll try to fix it next week.
Flags: needinfo?(pollti)
Comment on attachment 8584977 [details] [diff] [review]
patch, v3

I'm going to wait for the working patch, but this seems pretty good so far.  ;)
Attachment #8584977 - Flags: review?(bwinton)
Posted patch patch, v4 (obsolete) — Splinter Review
I'm sorry for my mistakes. Hopefully, this one will work properly.
Attachment #8584977 - Attachment is obsolete: true
Attachment #8584977 - Flags: review?(neil)
Flags: needinfo?(pollti)
Attachment #8588973 - Flags: review?(neil)
Attachment #8588973 - Flags: review?(bwinton)
Comment on attachment 8588973 [details] [diff] [review]
patch, v4

This looks OK by code inspection; I didn't actually apply the patch.
Attachment #8588973 - Flags: review?(neil) → review+
Comment on attachment 8588973 [details] [diff] [review]
patch, v4

Review of attachment 8588973 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me too!
Attachment #8588973 - Flags: review?(bwinton) → review+
Thanks!
What are the next steps here? / Is there anything I can do?
Put "checkin-needed" in the keywords, and someone will check the code in for you!
Keywords: checkin-needed
Thank you for working on this. There are several issues with the patch, please fix them and resubmit (feel free to ping me on IRC if you have questions). Output from the console:

unable to find 'suite/locales/en-US/chrome/messenger/messenger.properties' for patching
1 out of 1 hunks FAILED -- saving rejects to file suite/locales/en-US/chrome/messenger/messenger.properties.rej
patching file mailnews/base/util/nsMsgIncomingServer.cpp
Hunk #1 FAILED at 1607
1 out of 1 hunks FAILED -- saving rejects to file mailnews/base/util/nsMsgIncomingServer.cpp.rej
patch failed, unable to continue (try -v)
cleaning up working directory...done
abort: invalid date: 'Mid Apr 07 10:32:00 2015 +0100'
Keywords: checkin-needed
Posted patch patch, v5 (obsolete) — Splinter Review
Two fixes. Thanks, Archaeopteryx!
Attachment #8588973 - Attachment is obsolete: true
Posted patch patch, v6 (obsolete) — Splinter Review
Attachment #8594357 - Attachment is obsolete: true
Posted file patch, v7 (obsolete) —
[added missing whitspace]
Attachment #8594361 - Attachment is obsolete: true
Posted patch text as patchSplinter Review
Attachment #8594362 - Attachment is obsolete: true
http://hg.mozilla.org/comm-central/rev/bb995f40ce81
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
Are string changes for TB38 still allowed ?
Flags: needinfo?(rkent)
Ah, string changes. No, that's way too late.
Flags: needinfo?(rkent)
You need to log in before you can comment on or make changes to this bug.