Closed Bug 1643729 Opened 2 years ago Closed 5 months ago

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

Categories

(Thunderbird :: FileLink, enhancement)

enhancement

Tracking

(thunderbird_esr91 wontfix)

RESOLVED FIXED
97 Branch
Tracking Status
thunderbird_esr91 --- wontfix

People

(Reporter: dev, Assigned: TbSync)

References

Details

Attachments

(9 files, 9 obsolete files)

7.24 KB, image/png
Details
12.24 KB, image/png
Details
21.42 KB, image/png
Details
21.86 KB, image/png
Details
48 bytes, text/x-phabricator-request
Details | Review
452 bytes, image/png
Details
610 bytes, image/png
Details
13.83 KB, image/png
Details
13.85 KB, image/png
Details

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...

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

(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.

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.

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 ;)

I'd support the chance to edit the template. πŸ‘

(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?

@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.

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.

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.

Is good idea making editable filelink template!

(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:

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.

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.

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 :-)

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.

Depends on: 1740664
No longer depends on: 1740664

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?

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

(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

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.

Flags: needinfo?(tdulcet)

(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.

Flags: needinfo?(tdulcet)
Attached image 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?

Flags: needinfo?(dev)

Alesandro, any objections or other ideas?

Flags: needinfo?(alessandro)

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!!!

Summary: Allow to specify a "template" that FileLink adds to the Email → Add options to add expiry date and download limits in cloudFile template, and to indicate link is password protected

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?

Flags: needinfo?(alessandro)
Attached image 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)

Flags: needinfo?(alessandro)
Attached image paperclip.png (obsolete) β€”
Attached image 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.

Attached image cloud-attachment-protected.svg (obsolete) β€”

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.

Flags: needinfo?(alessandro) → needinfo?(richard.marti)

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?

Flags: needinfo?(richard.marti)

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.

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.

Attached image cloud-attachment-protected.png (obsolete) β€”
Attachment #9253390 - Attachment is obsolete: true
Attachment #9253513 - Attachment is obsolete: true
Attached image cloud-attachment.png (obsolete) β€”

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 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.

(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.

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?

(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!

Assignee: nobody → john

@ 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?

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

(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.

(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.

Attachment #9253609 - Attachment is obsolete: true
Attached image template-v3.png β€”

Templates with the new icons.

Attached image cloud-attachment.png (obsolete) β€”

Crispy icons are crispy when properly pixel hinted

Attachment #9253524 - Attachment is obsolete: true
Attachment #9253525 - Attachment is obsolete: true
Attached image cloud-attachment-protected.png (obsolete) β€”

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?

Attachment #9253620 - Attachment is obsolete: true

(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.

Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9254157 - Attachment description: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin → WIP: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin
Attachment #9254157 - Attachment is obsolete: true
Attachment #9254157 - Attachment is obsolete: false
Attachment #9254996 - Attachment is obsolete: true

NI myself so I remember to update those icons.

Flags: needinfo?(alessandro)
Attached image 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?

Attachment #9253706 - Attachment is obsolete: true
Attachment #9253707 - Attachment is obsolete: true
Flags: needinfo?(alessandro)
Attached image cloudfiles-32px.PNG β€”

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

How does it look in "Simple HTML"?

Attached image 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.

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?

(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.

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

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

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

Attachment #9254157 - Attachment description: WIP: Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin → Bug 1643729 - Add download limit, expiry date and option to indicate password protection to the cloud file link. r=mkmelin
Target Milestone: --- → 97 Branch

Pushed by mkmelin@iki.fi:
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

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED

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

Status: RESOLVED → REOPENED
Flags: needinfo?(john)
Resolution: FIXED → ---

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

Flags: needinfo?(john)

Pushed by alessandro@thunderbird.net:
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

Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Depends on: 1750943
Flags: needinfo?(dev)
You need to log in before you can comment on or make changes to this bug.