1643729 - Add options to add expiry date and download limits in cloudFile template, and to indicate link is password protected

Status: RESOLVED FIXED

(Thunderbird :: FileLink, enhancement)

Description

[datenheim]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:77.0) Gecko/20100101 Firefox/77.0

Steps to reproduce:

Simply add a file to a new message by using any FileLink providers.
You get some pre-defined code added to your message window that can not be affected by the user.
Especially clonky for HTML mode messages but also pure-text messages are affected.

Actual results:

The actual text/code added to the message editor after the successful upload of a FileLink attachment looks IMO "bulky".
I mean it takes up a lot of space and also might not integrate well with HTML templates (colours, text sizes etc.).
It also is fixed to english language no matter what language the current message is written in - which might interfere the communication flow.

Expected results:

It would be nice to have any measure to customize the code/text that is inserted in pure-text and HTML-mails.

Maybe there could be two text boxes in the configuration dialogue of the actual provider settings taking a html and text snippet for the text/code to be added to the Email. In this "template" a special {{anchor}} denotes the place where the actual link will be placed.

This basic "templating" would elegantly surpass design and language malaises.

As an option one might think to make the templates also email-account-specific, but that may go to far...

Comment 1

[Magnus Melin [:mkmelin]]

I agree we could improve that pre-defined code. Making it easily customizable sounds a bit too much though.

Comment 2

[Johannes Endres]

(In reply to datenheim from comment #0)

It also is fixed to english language no matter what language the current message is written in - which might interfere the communication flow.

It's not always english, but in the locale of the Thunderbird instance. Still it doesn't adapt to the message's language.

Comment 3

[Johannes Endres]

Now that bug 793118 is fixed, this is the most common feature request by users of my Addon (*cloud). Since I can't change the current "template" from an Addon (and being fluent in only 2 languages) I have to disappoint my users.

Comment 4

[steph-forum]

I love "cloud - FileLink for NextCloud and OwnCloud" because it's good for the planet as it allows to stop integrating heavy attachments in emails!

Indeed, it would be great to be able to modify the text of the links generated by this addon. This would allow us to indicate, among other things, the validity date of the links.

It would be so great that I registered on Bugzilla just to be able to write these lines. So I hope it will be possible ;)

Comment 5

[t.korth]

I'd support the chance to edit the template. 👍

Comment 6

[datenheim]

(In reply to Johannes Endres from comment #3)

Now that bug 793118 is fixed, this is the most common feature request by users of my Addon (*cloud). Since I can't change the current "template" from an Addon (and being fluent in only 2 languages) I have to disappoint my users.

Hello Johannes,
as far as I understand you my feature request can not be implemented into the plugin because the template used for the purpose is part of thunderbird itself?

Could you point out where it is located? Is it a separate file somewhere in the Thunderbird installation?

Comment 7

[Johannes Endres]

@datenheim True, the template is part of Thunderbird and the Addon has no chance to change it. (Actually there are about 60 templates as this is a localized string).

A quick grep shows that in a Thunderbird installation the template string is in omni.ja which is a zip file containing many others. So within the zip the actual file is composeMsgs.properties somewhere below chrome/, the actual path depending on the language.

I have no idea where the correcponding files are located in the source tree as I'm not working on Thunderbird itself.

Comment 8

[Simon Crowder]

I support the idea of making the FileLink Text an editable template.
I would especially like to see the inclusion of the expiry date and also of the required password for the shared files/folder to the outgoing email.

Comment 9

[S.Brooks]

I'd also like to support exposing the template where the user can change the text. The tag line "Mozilla Thunderbird makes it easy to share large files over email" is IMO unnecessary and tacky. I can see where a business might want to add a disclaimer or access duration notice.

Comment 10

[michele]

Is good idea making editable filelink template!

Comment 11

[datenheim]

(In reply to Johannes Endres from comment #7)

@datenheim True, the template is part of Thunderbird and the Addon has no chance to change it. (Actually there are about 60 templates as this is a localized string).

A quick grep shows that in a Thunderbird installation the template string is in omni.ja which is a zip file containing many others. So within the zip the actual file is composeMsgs.properties somewhere below chrome/, the actual path depending on the language.

I have no idea where the correcponding files are located in the source tree as I'm not working on Thunderbird itself.

So it looks like this topic could be closed here and should be (re)opened as a feature request for thunderbird itself.
Thanks Johannes for all the comments, and thanks for creating *link (o:

Comment 12

[Kris Lou]

This poster (http://forums.mozillazine.org/viewtopic.php?f=28&t=3058576) was able to override/replace the entire composeMsgs.properties with an edited version.

But his method is certainly targeted at a specific localization file, and very brute-force. And it is also prone to upgrade failures when composeMsgs.properties changes upstream.

Comment 13

[S.Brooks]

this still all applies to ESR 91.1.2

In omni.ja the code for injecting the file link and the styling for the HTML template is in

chrome\messenger\content\messenger\messengercompose\cloudAttachmentLinkManager.js

e.g. root.style.backgroundColor = "#D9EDFF";

the language template string "cloudAttachmentListFooter" is the line "Mozilla Thunderbird makes it easy to share large files over email"

As a brute force its possible to hack the code and save a modified omni.ja but this isn't sustainable and doesn't survive an update.

Perhaps someone that knows what they are doing could write a patch to contribute to Mozilla that would allow some of the styling elements (padding, background colour, footer string) to be read from the advanced preferences as a starting point rather than trying to create a custom template.

Comment 14

[datenheim]

S.Brooks, thanks for pointing that out.

(In reply to S.Brooks from comment #13)

...
Perhaps someone that knows what they are doing could write a patch to contribute to Mozilla...

Is there anybody out there that would like to be this somebody :-)

Comment 15

[John Bieling (:TbSync)]

Working on https://phabricator.services.mozilla.com/D130851 which adds the basics to be able to override template values. After that has landed, we can think about more options to tune the template. Looking at you, expiry date.

Comment 16

[John Bieling (:TbSync)]

With Bug 1627497 being accepted and ready to land, we now can provide values which should be used in the CloudFile template. As stated before, making the template fully customizable and letting the add-on provide a completely different template string, is not something we want to do.

What I think should be added:

• add an optional expiry date, which I would probably add inside the brackets, behind the size (if specified)

Anything else?

Comment 17

[S.Brooks]

Brilliant work!
Its not clear to me which parameters are already included in the existing scope for customising.

Noting that the ESR 91.3.1 removed the Mozilla spruke at the end which was my only real beef. I only use one provider so my needs are simple. Happy with decision not to support a fully customizable template.

Using Johannes Endres' excellent add-on with Cloudstor the customisation I would use:

• optional link expiry date if set as suggested
• background colour but existing is not offensive
• optional message at end (where the spruke was) to add disclaimer or company message

Comment 18

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #16)

What I think should be added:

• add an optional expiry date, which I would probably add inside the brackets, behind the size (if specified)

Anything else?

For my FileLink provider for Send add-on, I could use:

• Time limit/expiry date
• Download limit
• Boolean indicating if a password/authentication is required

Comment 19

[John Bieling (:TbSync)]

If a password is required, don't you have to mention that in the body of the email anyhow, because you need to tell where that password can be found? What is the gain of having the information that a PW is required in the attachment list? The user will get prompted for the password, so he will be notified.

Not saying we will not do it, just trying to understand the use case.

Comment 20

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #19)

If a password is required, don't you have to mention that in the body of the email anyhow, because you need to tell where that password can be found? What is the gain of having the information that a PW is required in the attachment list? The user will get prompted for the password, so he will be notified.

I thought it could be helpful for the recipient to be notified that a password is required before they click on the link. It could show a key icon/emoji or something similar after the size. 🔑 However, my add-on does not yet support password protecting the files, so this is the least important of the three things I listed.

The idea for password protecting the files is so that if the mail server is untrusted or if an attacker compromises the e-mail, they still would not be able to access the attached/FileLinked files. It provides another level of protection, in addition to the end-to-end encryption. The user would then need to send the password to the recipient using a different method and they probably would not want to mention this method in the e-mail body, as that would only help the attacker to know where to look. See bug 1516252 for more information about the use case of my add-on.

Comment 21

[John Bieling (:TbSync)]

Attachment: template.png

A mockup. I changed the paper clip icon to include a lock, if password protected. The icon has both alt and title attribute set to "Link is password protected."

Open question: Should the date be specified as a real Date object (and TB selects formating), or should it be a string and the add-on selects formating?

Comment 22

[John Bieling (:TbSync)]

Alesandro, any objections or other ideas?

Comment 23

[steph-forum]

Hello,

I am very happy to see that things are changing!

For my part in the frame containing the links to the attachments :
1- I add by hand and in grey just after the number of Kb: "- Link valid for 7 days; think of downloading the file to be able to find it ;-)"
2- I delete the Https:// link which is only used to show how to access my cloud...
3- I also delete the sentence "Mozilla Thunderbird allows to share large files easily".

It would be really great if I didn't have to do all this every time.

Thank you so much to everyone who is working on this!!!

Comment 24

[Alessandro Castellani [:aleca]]

Good improvements.
Icon
I'd prefer not to add a foreign looking icon with a different visual style. Would be better to have a variation of the clip icon with our lock icon, maybe colored orange.

Text
That grey text doesn't look WCAG complaint as it doesn't have enough contrast.
With a white background, the lightest we can go is #595959: https://webaim.org/resources/contrastchecker/?fcolor=595959&bcolor=FFFFFF

I'm not sure how digestible is that long string of text, how much flexibility do we have in organizing that data? Does it all have to be on one line all lowercase?

Comment 25

[John Bieling (:TbSync)]

Attachment: mock.png

When using multiple lines for the stats, how about using two different text colors? The lighter color is the #595959, the darker just "dark-grey".

Regarding the lock icon: I only found this in our tree:
https://searchfox.org/comm-central/source/calendar/base/themes/common/icons/locked.svg

But I could not get that to look good on the paper clip. If you have a spare moment, Alessandro, could you give it a try? I will attach the paperclip icon here as well (it is a data url in the source)

Comment 26

[John Bieling (:TbSync)]

Attachment: paperclip.png

Comment 27

[John Bieling (:TbSync)]

Attachment: plaintext.png

And this is how it looks in true plain text mode (created in plain text) and if the html version is viewed in "simple html" mode.

Comment 28

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment-protected.svg

I like the updated view, great work.
Here's the SVG I created of a protected attachment.
Adding Richard to this to be sure the svg is properly aligned to the grid and optimized.

Comment 29

[Richard Marti (:Paenglab)]

Comment on attachment 9253513 [details]
cloud-attachment-protected.svg

The actual paperclip for unprotected cloud files is different to this one. It has a flat top and isn't round like this. Shouldn't we be consistent?

Comment 30

[Alessandro Castellani [:aleca]]

We should also replace the cloud files clip to use our attach.svg icon.
It doesn't make sense having multiple variations of the same ideogram.

What I don't know is if is correct to use an SVG in this scenario, or if for compatibility reason, a PNG is better.

Comment 31

[John Bieling (:TbSync)]

I would go for a png, as the email can be opened by a different email client/web interface on the receiving end, which might not support (data-urls for) svg.

Comment 32

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment-protected.png

Comment 33

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment.png

Comment 34

[S.Brooks]

I'm sure you all know what you are doing :D but please check how the email look in Thunderbird after they are received. I do like the way the emails show up as having an attachment in the inbox (paperclip shown in the "sort by attachment" column) and I also like that the attachment is not a meaningful file (ie the attachment e.g. "Part 1.2" is NOT a picture of a paperclip or cloud which is good IMO) - small potatoes I know.

Comment 35

[Richard Marti (:Paenglab)]

Comment on attachment 9253524 [details]
cloud-attachment-protected.png

The stroke width is 1.5px which make the stroke blurry. Maybe better use 2px. The same applies to the unprotected icon.

Comment 36

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #21)

Open question: Should the date be specified as a real Date object (and TB selects formating), or should it be a string and the add-on selects formating?

My vote would be for TB to select the formatting, so that it would be constant for all add-ons and always be in the user's local format, maybe internally using Intl.DateTimeFormat(). I do not have a preference on the actual datatype, although the JS equivalent of Unix time (milliseconds since epoch) may be easier than a Date object, since those cannot be serialized.

(In reply to John Bieling (:TbSync) from comment #25)

Created attachment 9253389 [details]
mock.png

I am not sure if that expiry date was just a placeholder, but it should also show the time, as my add-on supports time limits as short as 5 minutes.

Comment 37

[John Bieling (:TbSync)]

In the current version of the patch, I opted for giving the add-on full control about the format (and yes, the used string in the mockup is a placeholder).

I decided like this, because some add-ons have strict time windows (like yours) where the time is needed. But others allow one or more weeks and then the time is not really needed and the string could just be needlessly long.

For the locale, you can use

browser.i18n.getUILanguage()

to get the locale of the user and use

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString

to get a Date string in the users locale.

Brain dump: There is however the spellchecker locale, which some have said should be used for the template, which we currently do not. If we ever implement that, I could forward the used spellchecker locale to the onFileUpload event and you could pick it up to format your time string.

In general, I try to give add-ons as much power as possible and avoid hardcoded stuff if possible.

Good?

Comment 38

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to John Bieling (:TbSync) from comment #37)

Brain dump: There is however the spellchecker locale, which some have said should be used for the template, which we currently do not. If we ever implement that, I could forward the used spellchecker locale to the onFileUpload event and you could pick it up to format your time string.

One can pass undefined to the locale argument of the Intl.DateTimeFormat() and toLocaleDateString()/toLocaleTimeString() functions, which I believe has the same effect as using browser.i18n.getUILanguage(). However, the spellchecker locale would be even better, so I would be happy to update my add-on to use that if TB ever implements it...

In general, I try to give add-ons as much power as possible and avoid hardcoded stuff if possible.

Good?

Well, if the date is less than a week away, you could show the date + time, otherwise just the date. Anyway, your proposed solution would be fine for my add-on. Thanks for implementing this, as it will be a huge improvement!

Comment 39

[John Bieling (:TbSync)]

Attachment: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin

Depends on D132300

Comment 40

[John Bieling (:TbSync)]

@ Alessandro : Thanks for your work on the icons! I added the patch for technical review. If the icon is to be changed, I can of course still do that. I will add new mockups shortly.

@ Richard : Do you want to check the new html code and want to be added as reviewer?

Comment 41

[Richard Marti (:Paenglab)]

(In reply to John Bieling (:TbSync) from comment #40)

@ Richard : Do you want to check the new html code and want to be added as reviewer?

Magnus should be enough.

Comment 42

[John Bieling (:TbSync)]

(In reply to S.Brooks from comment #34)

I'm sure you all know what you are doing :D but please check how the email look in Thunderbird after they are received. I do like the way the emails show up as having an attachment in the inbox (paperclip shown in the "sort by attachment" column) and I also like that the attachment is not a meaningful file (ie the attachment e.g. "Part 1.2" is NOT a picture of a paperclip or cloud which is good IMO) - small potatoes I know.

This patch only improves/extends the html/text which is added to the email. It does not touch the fake attachments added. That is being discussed in bug 1670791.

Comment 43

[John Bieling (:TbSync)]

Attachment: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin

Depends on D132300

Comment 44

[John Bieling (:TbSync)]

Attachment: template-v3.png

Templates with the new icons.

Comment 45

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment.png

Crispy icons are crispy when properly pixel hinted

Comment 46

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment-protected.png

Comment 47

[John Bieling (:TbSync)]

Comparing the first and the second version of the icons, I think I like the first version better. The lock symbol is now a little bit narrower and for me the paper clip does not look as elegant as before. Would it be ok to stick with the first version?

Comment 48

[John Bieling (:TbSync)]

Attachment: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin

Depends on D133106

Comment 49

[Alessandro Castellani [:aleca]]

(In reply to John Bieling (:TbSync) from comment #47)

Comparing the first and the second version of the icons, I think I like the first version better. The lock symbol is now a little bit narrower and for me the paper clip does not look as elegant as before. Would it be ok to stick with the first version?

Yeah, I'll work on this a little bit more later today.
I can keep the lock size consistent and pixel hinted.

Comment 50

[John Bieling (:TbSync)]

Attachment: WIP: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin

Comment 51

[Alessandro Castellani [:aleca]]

NI myself so I remember to update those icons.

Comment 52

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment.png

Since all our icons are 16px, making a 24px icon is a bit weird and doesn't work very well if we want to keep the same style and pixel hint, without making it look chunky.
I made these 32px, how do they look? Too big?

Comment 53

[Alessandro Castellani [:aleca]]

Attachment: cloud-attachment-protected.png

Comment 54

[John Bieling (:TbSync)]

Attachment: cloudfiles-32px.PNG

I think the bigger size is ok. What do you think?

Comment 55

[Richard Marti (:Paenglab)]

How does it look in "Simple HTML"?

Comment 56

[John Bieling (:TbSync)]

Attachment: cloudfiles-32px-simple.PNG

Simple view is not really style-able, this is the best I could do. Plain text is:

I've linked 2 files to this email:

  * policies.json
    <https://cloud.bieling.family/index.php/s/C1nGxVfPjN4vebB/download>
    Size: 56 bytes
    Expiry Date: 01.01.2022
    Download Limit: 1
    Host: BielingCloud
    Password Protected Link:
    https://cloud.bieling.family/index.php/s/C1nGxVfPjN4vebB/download

  * index.png <https://cloud.bieling.family/index.php/s/1zMbcpw2L4qjE0W/download>
    Size: 398 bytes
    Expiry Date: 01.01.2022
    Download Limit: 1
    Host: BielingCloud
    Link: https://cloud.bieling.family/index.php/s/1zMbcpw2L4qjE0W/download

In a pure plain text email the first link behind the file name is not added.

Comment 57

[John Bieling (:TbSync)]

While working on this bug, Magnus asked me to remove the link behind the service provider icon in each attachment entry. This made the plain text version look better (less links) and reduced redundancy. The link was either the value of the service_url manifest entry or the value specified in the templateInfo.

I am now looking for a way to get that information back into the email (if specified/enforced by the add-on). I was thinking about a string below all attachment entries, still in the blue area, which spells something like:

Learn more about the used cloud service: BielingCloud, Box, GoogleDrive

(Simulating multiple different service providers used).

In HTML each service name would link to the service url, in plain text this would be a list like:

Learn more about the used cloud service: 
 * BielingCloud: <url>,
 * Box: <url>,
 * GoogleDrive: <url>

What do you think? Alternative wording?

Comment 58

[S.Brooks]

(In reply to John Bieling (:TbSync) from comment #57)

I am now looking for a way to get that information back into the email (if specified/enforced by the add-on). I was thinking about a string below all attachment entries, still in the blue area, which spells something like:

Learn more about the used cloud service: BielingCloud, Box, GoogleDrive

(Simulating multiple different service providers used).

Terrible idea and a step backwards IMO.

Why? IMO the advertising is a blight and has no place in my emails. The only link should be to the file that is relevant to the reader. The reader can do their own research if they want to find a cloud platform. STRONG VOTE NO.

Comment 59

[John Bieling (:TbSync)]

@S.Brooks: You can enable/disable the footer/links in the add-on options. This is about optional features.

Comment 60

[Magnus Melin [:mkmelin]]

Are the add-ons required to have that option? From a user POV I really doubt many people want to have such a link.

Comment 61

[John Bieling (:TbSync)]

I will make it such, that it is off by default.

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/70a392298b42
Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin

Comment 63

[Magnus Melin [:mkmelin]]

Backed out for test failures and fluent linting failure. (Please run ./mach lint --config-path=comm/tools/lint -l fluent-lint comm/*)

https://treeherder.mozilla.org/logviewer?job_id=363228393&repo=comm-central

Backout: https://hg.mozilla.org/comm-central/rev/34188ef8b01c71a68825d58902d7522a2baf44d2

Comment 64

[John Bieling (:TbSync)]

Intl.format is of course not defaulting to German on our main CI. I will adjust the test and check with a try run.

Comment 65

[John Bieling (:TbSync)]

Try run:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=516e2c65d0b22a648d3e646efeb30578cf4ec3cf

Comment 66

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a8789c8d6692
Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin