newmail_notifier: Include sender and subject in desktop notification #9492
Conversation
| ? ' +' +otherCount | ||
| // No sender name, e.g. "New Mail! (3) | ||
| : '('+otherCount + ')'; |
There was a problem hiding this comment.
Please add spaces around the plus-signs. Or maybe you could rework these title-related lines a little, including line 40, to be a little less terse but more readable?
| if ($latest_uid !== null) { | ||
| $headers = $storage->fetch_headers($mbox, [$latest_uid]); | ||
| // fetch_headers returns an array, but we only care about the first one. | ||
| $headers = array_shift($headers); |
There was a problem hiding this comment.
Not a big deal, but here you could use array_first().
| $headers = array_shift($headers); | ||
| if ($headers !== null) { | ||
| $from = $headers->from; | ||
| $subject = $headers->subject; |
There was a problem hiding this comment.
I wonder what sanity checks we should apply here. We probably should not use too long subjects. Also we should parse the From header content. We definitely should decode the content, not pass as-is.
See program/actions/mail/show.php for hints on what we normally do with subject and from header.
There was a problem hiding this comment.
Thanks, I'll take a look. I wasn't sure whether to limit the subject length here or on the client side.
There was a problem hiding this comment.
We probably should not use too long subjects.
I tried with a long subject, and KDE makes them scrollable:
I haven't tested with Windows yet, but I think it truncates them.
We definitely should decode the content, not pass as-is. See program/actions/mail/show.php for hints on what we normally do with subject and from header.
Thanks for the pointer. I thought the headers would already be decoded here.
| var otherCount = prop.count - 1; | ||
| if (otherCount >= 1) { | ||
| title += prop.from | ||
| // Have sender name, e.g. "Daniel L +3" |
There was a problem hiding this comment.
I don't like this whole thing. Neither "Daniel L +3" looks good nor "New Mail! (3)".
How about we show the sender/subject only when there's a single new mail message? And "(n) New Mail!" in other cases?
|
I haven't had free time to revise this yet, but I'll get back to it soon. |
|
Updated to properly decode the from and subject headers. I updated it to just show the sender name rather than their email address too, to make the notification a bit more concise. Tested with a multi-byte (Chinese) subject: Tested with a Chinese sender name: |
- Show "(2) New Mail!" when there's more than one email - Decode from and subject headers Signed-off-by: Daniel Lo Nigro <[email protected]>
| $from_details = array_first( | ||
| rcube_mime::decode_address_list($headers->from, 1, true, $headers->charset) | ||
| ); | ||
| $from = $from_details['name']; |
There was a problem hiding this comment.
Name can be empty. In such case let's fallback to the email address. Also, in some corner cases From header might not exist at all.
Displays the sender and subject in desktop notifications:
References #8360 #7826 #8587