1146587 - Attachment reminder keywords miss .docx, .xlsx, .pptx / .zip, .jpg, .jpeg (and .odt, .ods, .odp from bug 633818)

Status: NEW

(Thunderbird :: Message Compose Window, enhancement)

Description

[jschoett]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2334.0 Safari/537.36

Steps to reproduce:

Compose new e-mail, insert recepients and subject, insert text containing ".docx" to describe the file attachement.


Actual results:

No attachment reminder comes up.


Expected results:

The attachment reminder should come up, like it should, when the text contains ".doc".

Same applies for ".xlsx" and ".pptx" as these are the wide-spreaded current MS Office file formats.

Comment 1

[jschoett]

tarball "thunderbird-31.5.0.source.tar.bz2" (comm-esr31)

it seems to be in this file:
./mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties

it's the property key "mail.compose.attachment_reminder_keywords"

a patch:

--- ./mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties	Wed Feb 25 18:14:11 2015
+++ ./mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs1.properties	Mon Mar 23 18:03:27 2015
@@ -290,7 +290,7 @@
 ## Attachment Reminder
 ## LOCALIZATION NOTE (mail.compose.attachment_reminder_keywords): comma separated
 ##   words that that should trigger an attachment reminder.
-mail.compose.attachment_reminder_keywords=.doc,.pdf,.xls,.ppt,.rtf,.pps,attachment,attach,attached,attaching,enclosed,CV,cover letter
+mail.compose.attachment_reminder_keywords=.doc,.docx,.pdf,.xls,.xlsx,.ppt,.pptx,.rtf,.pps,attachment,attach,attached,attaching,enclosed,CV,cover letter
 
 addAttachmentButton=Add Attachment…
 addAttachmentButton.accesskey=A

Comment 2

[Thomas D. (:thomas8)]

Thanks for filing & patch! :)

Sounds good to me. Obviously missing. ui-r+ from me for the idea if there are no transition issues.

With this patch applied, how is the transition experience for users with existing customized keyword lists? (I guess they will just not be updated, but at least we'll catch all new installations).

Can you pls "add an attachment" here with your patch as a file like bug1146587.patch?

OT:

1) Jschoett, as a user of attachment reminder, have you already discovered that you can activate the attachment reminder manually at any time (even without keywords-trigger), using "Remind me later" from Attachment button, or File > Attach > Remind me later, or attachment pane context menu?

2) Do you know that you can activate attachment reminder even when you already have some files attached, and that it will not automatically be deactivated by adding attachments (so it's like an alarm clock which will definitely ring at the defined time unless you explicitly switch it off before)?

3) Are you aware that active attachment reminder will reliably be saved with your draft, so it's almost like a feature of "Attach when sending" which can assist to send the latest version of a file?

Comment 3

[jschoett]

Attachment: bug1146587.patch

Comment 4

[jschoett]

Comment on attachment 8583331 [details] [diff] [review]
bug1146587.patch

Here is the patch.

I was not aware of this three reminder features. Thanks :)

Comment 5

[Thomas D. (:thomas8)]

While we are here, can we also add these?

Open Office File formats (an ISO International Standard format for office documents)
.odt (word processor documents)
.ods (spreadsheet documents)
.odp (presentation documents)

.zip (Zip archives)
.jpg (Photos)

I'll assume that this won't hurt performance too much?

Of course we can't add ALL the file extensions out there, but are there any other candidates that are likely to be added in general conversation, and that are likely to be attached later when sending (e.g. because user is doing final touches to them while composing)?

Comment 6

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #2)

> With this patch applied, how is the transition experience for users with
> existing customized keyword lists? (I guess they will just not be updated,
> but at least we'll catch all new installations).

Aceman, can you comment what happens to user-customized prefs when we change the default value?

Are we technically able to add things to the user-customized pref?
Should we (in this particular case)? Or does it violate good practice to not touch user-defined stuff?

As anecdotal user evidence, I have added some attachment reminder keywords in my German language version of TB. I wouldn't want to lose them when I update TB, but I would love to benefit automatically from the new file extensions this bug wants to add as reminder keywords.

Comment 7

[:aceman]

Well, the list is in /mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties which means it is locale dependent. The patch adds the new file extensions for en-US. It depends on the localizer if he also picks the change up for his language.

Second, if you have changed the pref value in your TB installation (profile) it will not change even if the default value changes (unless it changed to the exact same value you have in your profile).

We could have a list of user customized values in a separate pref (that would be merged with the default list when evaluating), but then how to solve the case when the user wants to remove a word from the default list?

Comment 8

[Ian Neal]

(In reply to :aceman from comment #7)
> Well, the list is in
> /mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
> which means it is locale dependent. The patch adds the new file extensions
> for en-US. It depends on the localizer if he also picks the change up for
> his language.
> 
That does also mean that each time it is changed, because the entity name isn't changed, a post to the l10n list should be made so that localisers do get the chance to pick it up.

Comment 9

[Kent James (:rkent)]

Aren't these filetypes pretty universal, independent of locale? If seems to me to be a flawed design if we are including strings which are really identifiers for content types in the locales. Perhaps you could add a list on non-localized file types, and append those to the localized version?

Comment 10

[Thomas D. (:thomas8)]

(In reply to Kent James (:rkent) from comment #9)
> Aren't these filetypes pretty universal, independent of locale? If seems to
> me to be a flawed design if we are including strings which are really
> identifiers for content types in the locales. Perhaps you could add a list
> on non-localized file types, and append those to the localized version?

Yes... as aceman said in comment #7:
> We could have a list of user customized values in a separate pref (that
> would be merged with the default list when evaluating), but then how to
> solve the case when the user wants to remove a word from the default list?

I like your idea of having two separate prefs for the localized keywords and non-localized file types.

But I would like to err in favor of customizability, because we never know what circumstances might make private or corporate users want to adapt (esp. reduce) even the non-localized list of file types.

Perhaps, with a little effort, we could have the best of both worlds:

- non-localized pref with default file types
- separate, localized pref with localized keywords
- in keyword customization dialogue, show full combined list of both types as we do now
- any keywords added are added to the localized pref (even file types, after checking if they don't already exist in the non-localized pref)
- if a file type keyword (starting with a dot) is removed, we first try to remove it from non-localized pref; if not found there, we try remove it from localized pref.

that way, we'd ensure that the non-localized pref remains untouched for most cases, which means we can update it as required and the updates will be effective for most users.

I'm also wondering if during updating of TB, we could somehow find out if the customized string still contains all the default values, and if yes, we could have a silent once-only routine to add any new keywords. If users only added keywords to the default set and removed none, they'll certainly want any new default keywords we decide to include.

Btw, the whole system is still flawed because it does not take the actual composition language into account, bug 545837.

Comment 11

[:aceman]

Bwinton, mkmelin, what do you think about the plan from Thomas?

Comment 12

[Magnus Melin [:mkmelin]]

Yeah i suppose it would make sense to separate out the file types as another pref.

Comment 13

[:aceman]

(In reply to Ian Neal from comment #8)
> That does also mean that each time it is changed, because the entity name
> isn't changed, a post to the l10n list should be made so that localisers do
> get the chance to pick it up.
Yes, for the file extensions we had to do that. But for the real words, if we add something for en-US, it does not mean localizers need to pick it up. We could assume localizers already put all the needed words in the pref (or can change them any time). The number of words in localized versions do not need to match en-US.

Comment 14

[:aceman]

jschoett, are you able to implement what is proposed in comment 10?
Also notice people have come to a similar proposal in bug 633818. So I am now not sure in which bug to implement this feature. Both bugs are only about adding specific extensions.

Comment 15

[jschoett]

(In reply to :aceman from comment #14)
> jschoett, are you able to implement what is proposed in comment 10?

Sorry, I don't have enough time these days. If no one else takes this task, I can maybe grab it in the summer holiday. But I have not contributed to TB yet and I also have not tried to make/compile the project (in order test changes)...

When we start to implement, I would suggest to create a new issue and link the related issues to it.

Comment 16

[Blake Winton (:bwinton) (:☕️)]

I'll go along with what Magnus said.

Comment 17

[:aceman]

OK, I can look at this. I try to implement the backend in bug 633818 (there is already WIP patch) and then add the new extensions here.

Comment 18

[Wayne Mery (:wsmwk)]

Thomas, remember comment 10?

Rob, Magnus, see any complications or challenges to achieve localization via pref ?

FWIW, https://mzl.la/3lV9ICY is a full list of attachment bug reports

Comment 19

[Magnus Melin [:mkmelin]]

Yes it's complicated, because we allow customizing these words by the user. Then save the list they wanted. If we now have two prefs how would we know where to save.
It could be best not to allow UI for user customization of the known file extensions, but limit it to the words only.

Comment 20

[Rob Lemley [:rjl]]

I don't have anything to add here.

Comment 21

[Thomas D. (:thomas8)]

This would appear to be a duplicate of Bug 633818, but unfortunately both bugs also have relevant discussions about bigger plans of changing the backend and UI. For now, keeping both bugs open.

Comment 22

[Nikki Taylor]

Attachment: same problem

Thanks from reublogolup1974.