Opened 12 years ago
Closed 7 years ago
#10743 closed enhancement (fixed)
[Patch] Email addresses should be mailto links
Reported by: | Ryan J Ollos | Owned by: | Ryan J Ollos |
---|---|---|---|
Priority: | normal | Component: | AccountManagerPlugin |
Severity: | minor | Keywords: | |
Cc: | Trac Release: |
Description
Inspired by #6259, transform the email addresses into mailto links.
Attachments (1)
Change History (18)
comment:1 Changed 12 years ago by
comment:2 follow-up: 12 Changed 12 years ago by
Severity: | normal → minor |
---|
This might be most useful for user lists, if shown by the UserQuery
macro that re-uses this template.
But there is a problem, that we don't guarantee to return valid email addresses. Note that all addresses are piped through Chrome.format_author
, and we may need to at least check for tailing '...' before formatting them blindly afterwards, right?
comment:3 Changed 12 years ago by
Yes, both of those seem valid and neither were on my radar. I will investigate and rework the patch.
comment:4 Changed 12 years ago by
Hold on, please. I'm about to push some related changes, and might finish them tonight.
I've refocused on the combined account verification concept, including new approval/ban feature, as I mentioned before. Therefore the code is significantly different from yours, and you'd better have a look at it, before investing more time.
comment:5 Changed 12 years ago by
Okay, I'll refocus elsewhere and wait for your changes. No rush on my part, just want to efficiently invest my time, so thanks for helping me make sure I am doing that by syncing my efforts with yours. I'll send you an email with a proposed plan for my work, so that we can better coordinate our efforts.
comment:6 follow-up: 7 Changed 12 years ago by
I think this feature still has value and remains unimplemented in the latest revision of the codebase. I'd care to rework the patch provided, with attention to the details noted in comment:2. Does that sound good?
Later on, it might even be nice to have the email address link to a small form, so that messages can be sent directly from Trac, thereby originating from the standard Trac notification sender email address and not requiring a client-side email client for sending messages. That seems like too much effort for now, but is a feature I would make use of if available.
comment:7 follow-up: 8 Changed 12 years ago by
Replying to rjollos:
I think this feature still has value and remains unimplemented in the latest revision of the codebase. I'd care to rework the patch provided, with attention to the details noted in comment:2. Does that sound good?
Fine for me.
Later on, it might even be nice to have the email address link to a small form, so that messages can be sent directly from Trac, thereby originating from the standard Trac notification sender email address and not requiring a client-side email client for sending messages. That seems like too much effort for now, but is a feature I would make use of if available.
May I suggest to add such a feature to AnnouncerPlugin instead? That plugin might be able to provide a generic mail form on-top of every email address, given it has not been obfuscated (or even then while hiding the real address and just allowing to send), if a special permission EMAIL_* has been granted to the user. Especially that last bit, combined with SpamFilterPlugin/captcha/whatever support could make a really valuable addition. At least I see use for it the longer I think about it.
But for what its worth I would not dare to implement anything nearly as feature complete as on other platforms for now - think sent-box, internal user-to-user communication. Just keep it for starting communication - i.e. the sender must provide a valid email for reply.
comment:8 follow-up: 9 Changed 12 years ago by
Replying to hasienda:
May I suggest to add such a feature to AnnouncerPlugin instead? That plugin might be able to provide a generic mail form on-top of every email address, given it has not been obfuscated (or even then while hiding the real address and just allowing to send), if a special permission EMAIL_* has been granted to the user. Especially that last bit, combined with SpamFilterPlugin/captcha/whatever support could make a really valuable addition. At least I see use for it the longer I think about it.
Yes, that sounds like an excellent idea, and would allow for some other scenarios I had in the back of my mind.
But for what its worth I would not dare to implement anything nearly as feature complete as on other platforms for now - think sent-box, internal user-to-user communication. Just keep it for starting communication - i.e. the sender must provide a valid email for reply.
Yes, send-only seems like a suitable scope for the feature. By default [announcer] email_replyto
could be used for setting the reply address, but we could also offer the sender the opportunity to add their own reply-to, to support cases such as multiple administrators for a Trac instance (oh, but also that the sender may not be an administrator! ... I'll need to think this through more completely).
comment:9 follow-up: 10 Changed 12 years ago by
Replying to rjollos:
Yes, send-only seems like a suitable scope for the feature. By default
[announcer] email_replyto
could be used for setting the reply address, but we could also offer the sender the opportunity to add their own reply-to, to support cases such as multiple administrators for a Trac instance (oh, but also that the sender may not be an administrator! ... I'll need to think this through more completely).
But of course we could just use the email address that the user has set in their preferences as the reply-to, which would make even more sense. This was probably obvious to you from the start!
comment:10 Changed 12 years ago by
Replying to rjollos:
But of course we could just use the email address that the user has set in their preferences as the reply-to, which would make even more sense. This was probably obvious to you from the start!
Well, this has been a strong option. Now, that you talk about it, it might be good to require it (or even require it verified verified) before allowing to use this feature at all. Not only saves an extra email address input field in the email sent form, but provides an essential protection against foreseeable abuse.
comment:11 Changed 12 years ago by
I've been using Git a lot lately, posting my Trac patches to GitHub and very recently (#11071) even started posting patches for trac-hacks plugins to GitHub. I seem to have a good workflow now, seem to be moving past what I've found to be a fairly steep learning curve and I'm finding it easier to work with Git now than working with clunky patch files. It also open the possibility of using GitHub's code review tools on patches (for example, you can directly comment on a diff).
I just wanted to check and make sure it would work for you if I post patches, such as the one for this ticket, to branches on GitHub. I know you are an Hg user, and though I've used the git-svn connector a bit now, I've never used Hg or an Hg-Git connector. I have hardly any experience with Hg, and feel the need to gain a bit more experience with Git before trying to add another DVCS to my toolset.
comment:12 Changed 12 years ago by
Replying to hasienda:
But there is a problem, that we don't guarantee to return valid email addresses. Note that all addresses are piped through
Chrome.format_author
, and we may need to at least check for tailing '...' before formatting them blindly afterwards, right?
Yes, I see now. There are so many parameters governing whether the email address will be displayed that I don't see an easy way to sort it out through the session and configuration. The parameters include at least EMAIL_VIEW
permission, [trac] show_email_addresses
and [trac] never_obfuscate_mailto
. It would be nice if Trac always wrapped an obfuscated email with <span class="obfuscated"> </span>
, but it will be easy enough to check for the unicode character "…".
Changed 12 years ago by
Attachment: | t10743-r13241-1.diff added |
---|
comment:13 follow-up: 14 Changed 12 years ago by
Patch in t10743-r13241-1.diff. I'd like to add some macro test cases, which will help with #11119 as well. I have in mind to use a test harness like [13129]. I would go ahead and write a patch that adds the test harness, but I want to check with you first to be sure that you are thinking similarly before making the effort.
comment:14 Changed 12 years ago by
Replying to rjollos:
I would go ahead and write a patch that adds the test harness, but I want to check with you first to be sure that you are thinking similarly before making the effort.
Sure, why not? I like unit testing. Just give me more, really. :-)
comment:15 Changed 8 years ago by
Owner: | Steffen Hoffmann deleted |
---|
comment:16 Changed 7 years ago by
Owner: | set to Ryan J Ollos |
---|---|
Status: | new → accepted |
Implemented in ticket:10741:th10741-r12501-1.patch