I see two options:

1. Have the same UI as the inline composer (with text formatting options up top).
2. Allow straight HTML for more complex signatures (corporate email often requires a little HTML table with images and links and everything).

The second option actually seems the simplest as it allows very simple plain text signatures and the most complex of HTML signatures, but I'd be happy with either. :)

$100 bounty added here: https://www.bountysource.com/issues/6659384-html-signatures

Anyone who takes this on should read over bug #712895, especially comment 2 from Michael Gratton.  The ticket itself is a nice discussion of the various issues surrounding email signatures.

That said, I do not have any concern at the moment for supporting .signature files and the patch for this ticket wouldn't need to worry about that.

I do encourage anyone who takes this on to discuss their approach here first before jumping in, as there's a lot of approaches and a lot of pitfalls with this feature.

Created attachment 291955 [details] [review]
Detect HTML signatures, and insert them without escaping

I'm bad at reading instructions, so I'm going to go ahead and post a 
simple solution.  This attempts to determine if a signature is HTML or 
plain text, and escapes as appropriate.  It does this by looking for 
HTMLish tags, and it assumes that any with them must by HTML.

Now, for some discussion:

1) Should this be automatically detected or not?  The nice thing about 
automatic detection is that we avoid additional widgets in the accounts 
dialog, which can already be too large in some cases.  Most of the time, 
it'll just do the right thing, but it could occasionally mistrigger, 
which would be surprising.  The good news is that the user will see it 
in their composer, so they won't be sending out messages without seeing 
their broken sigs.

2) Should .signature files be able to contain HTML?  Right now, they 
can, because it was easier.  But do people expect this behavior?  It may 
be safer to assume that these are plain text.  Perhaps we could look for 
.signature.html for an HTML one.

3) Should we preserve whitespace in HTML signatures?  This patch 
doesn't, but it'd be very easy to do.  This would let people easily add 
a link to a signature without needing to insert <br />s for linebreaks.  
But it could confuse people who don't expect line breaks to matter in 
HTML.

This approach is certainly the shortest line between two points.  If we were to go this route, we should note in the dialog that HTML is accepted, since we're not offering an HTML editor for entering it.

To answer your questions:

1) I'm not terribly worried about the autodetection failing, but I suppose someone with a clever signature would fail, i.e.:

-- Jim
<disclaimer>
  All opinions are mine and mine alone.
</disclaimer>

The troubling part of this is that there's no method for disabling autodetection, meaning the user would simply need to stop using this signature to get the right results.

2) Regarding .signature files, we can do nothing but bow to legacy practice and assume they can contain HTML.  Yes, that's frustrating, but for the purposes of this patch I would say that linking your .signature to Geary's signature file is a power-user thing, i.e. use at your own risk.

3) I'm kind of mixed minds regarding whitespace.  This is probably the best argument for offering an HTML editor for the HTML signature, as it offers WYSIWYG semantics.

I haven't actually played too much with this patch, Robert (slammed right now), but how does this deal with appending the signature to the plain text version of the message (or does it deal with that at all)?

It looks like Thunderbird has a "Use HTML" checkbox for their signature editor, but the user is required to enter the HTML by hand (no WYSIWYG).  That solves the auto-detect problem but feels unsatisfying to me.  In 2014, I like to believe the user doesn't have to enter HTML no more than they should be expected to hand-code RTF.  But, in the interests of gradual improvements, we might have to take that.

But I wouldn't mind having such a checkbox (which would also solve the .signature file question -- if it has HTML, check that box).  As far as the account dialog, I could see switching between the multiline editor and a WebKit editor.  Or, like the composer, always use a WebKit editor that switches from plain to rich text mode depending on the checkbox.

Incidentally, here's what I'm seeing when I set my sig to "<b>--Jim</b>" in Thunderbird (source trimmed):

Content-Type: multipart/alternative;
 boundary="------------020808090105060303020208"

This is a multi-part message in MIME format.
--------------020808090105060303020208
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

testing.

-- 
*-- Jim*

--------------020808090105060303020208
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: 7bit

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=utf-8">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    testing.<br>
    <br>
    <div class="moz-signature">-- <br>
      <b>-- Jim</b></div>
  </body>
</html>

--------------020808090105060303020208--

... so, the bold text was marked down to *-- Jim* in the plain message portion and given its own DIV in the HTML.  I don't think either is required for our purposes, but does indicate that the HTML was stripped for the plain text message.  That seems like a hard requirement for us.

I didn't play with this any further, but toying with Thunderbird might give us some answers to your questions (and any more that come up) for no other reason than they've been down this road already.

(In reply to comment #5)
> This approach is certainly the shortest line between two points.  If we were to
> go this route, we should note in the dialog that HTML is accepted, since we're
> not offering an HTML editor for entering it.

Could this be a hover hint for the text entry?  Regardless, it should get noted in the help files.

> To answer your questions:
> 
> 1) I'm not terribly worried about the autodetection failing, but I suppose
> someone with a clever signature would fail, i.e.:
> 
> -- Jim
> <disclaimer>
>   All opinions are mine and mine alone.
> </disclaimer>
> 
> The troubling part of this is that there's no method for disabling
> autodetection, meaning the user would simply need to stop using this signature
> to get the right results.

You could do this, but you'd have to do it as HTML:

-- Jim<br />
&lt;disclaimer&gt;<br />
...

> 3) I'm kind of mixed minds regarding whitespace.  This is probably the best
> argument for offering an HTML editor for the HTML signature, as it offers
> WYSIWYG semantics.

I feel like auto whitespace would be good in the text entry but bad for .signature files, but doing different things could be confusing here.

> I haven't actually played too much with this patch, Robert (slammed right now),
> but how does this deal with appending the signature to the plain text version
> of the message (or does it deal with that at all)?

I doesn't do anything different from how they were handled already.  Remember, the composer handles everything as HTML, and it generates the plain text version as it's sending the email.  At this point, it has no idea that a bit of HTML corresponds to the signature.

I'm not looking forward to adding a WYSIWYG editor for the signature.  It'd mean we need a toolbar with formatting options, a link dialog, etc.  I fear it'll outgrow the accounts dialog and need it's own dialog.

Trying to think laterally, what if we allowed the user to select some text in the composer, right click, and set that as the signature?  It would allow us to take advantage of the existing editor.  It's admittedly a non-obvious mechanism, but this isn't something you need to do that often.

(In reply to comment #6)
> Could this be a hover hint for the text entry?  Regardless, it should get noted
> in the help files.

Yeah, that could be one approach.  Putting in the help docs is fine, but my experience is that that's the last place people look for help (if they even think to look there at all).  A tooltip, dimmed text when the box is empty, or a small note above/below the box -- any of these would be fine.

> You could do this, but you'd have to do it as HTML:

You're right, of course.

> Remember,
> the composer handles everything as HTML, and it generates the plain text
> version as it's sending the email.

Yes, I forgot -- well, then that decision is paying off dividends yet again.  Good!

> I'm not looking forward to adding a WYSIWYG editor for the signature.  It'd
> mean we need a toolbar with formatting options, a link dialog, etc.  I fear
> it'll outgrow the accounts dialog and need it's own dialog.

That's what I'm thinking of too.  I'm torn because it seems like a lot of work just to generate a signature (which isn't something one fiddles with too much, if more than once), but sig support has been such a heavy feature request from users, there's a part of me that says we should do it right.

> Trying to think laterally, what if we allowed the user to select some text in
> the composer, right click, and set that as the signature?

I think that would be a nice feature, but it can't be the primary way of setting a signature.

Looking at Thunderbird, they don't offer a WYSIWYG editor for HTML signatures.  I always like to think we can do better, but we don't have a surplus of manpower either.

Ok, here's what I'm thinking at the moment.  Just to put some darts in the dartboard:

* Plain text editor
* Tooltip for text box: "Markup will be treated as HTML, i.e. <B>, <I>, etc." (i.e. autodetect HTML)
* This means we escape HTML (i.e. ampersands) *unless* we detect markup, in which case the entire signature must be valid HTML.  (Right?)
* Auto-insert whitespace.  Let's roll with it.  Without that, people will screw up multiline signatures every time.  I know I will.

One question from this:

* Can we show a preview of the HTML signature?  I *really* like how Stack Overflow et al. displays a preview of your Markdown.  That would be cool.  It also means the user doesn't have to type their signature, press Save, open composer, see it's not quite right, open Accounts, edit, Save...

But if we did, I suspect we'll need to break out the signature editor to its own dialog, so ... trade-offs.

* Is there any markup we strip or disallow from signatures?  I can't think of anything off the top of my head or even why we would want to disallow it other than it means the user can insert arbitrary markup into the composer.  Javascript is disabled, so no worries there, but is it possible they'll break something in the composer if we don't?  (For example, could they break the composer w/ embedded CSS?)

(In reply to comment #7)
> > Trying to think laterally, what if we allowed the user to select some text in
> > the composer, right click, and set that as the signature?
> 
> I think that would be a nice feature, but it can't be the primary way of
> setting a signature.

The existing box would still be there.  This would just be a way of adding WYSISYG editing without adding a whole new editor.  A user could even go between them -- set the text in the accounts dialog, add links in the composer, and then save the resultant thing as the new signature.  But I agree it's slightly awkward, and I'm not entirely sure I like it.

> * Plain text editor
> * Tooltip for text box: "Markup will be treated as HTML, i.e. <B>, <I>, etc."
> (i.e. autodetect HTML)
> * This means we escape HTML (i.e. ampersands) *unless* we detect markup, in
> which case the entire signature must be valid HTML.  (Right?)
> * Auto-insert whitespace.  Let's roll with it.  Without that, people will screw
> up multiline signatures every time.  I know I will.

Should we do this for HTML .signature files as well?  If we do, we should probably careful and not replace "<br />\n" with "<br /><br />".

> * Can we show a preview of the HTML signature?  I *really* like how Stack
> Overflow et al. displays a preview of your Markdown.  That would be cool.  It
> also means the user doesn't have to type their signature, press Save, open
> composer, see it's not quite right, open Accounts, edit, Save...

One way to do this would be to replace the text box with a webview to preview the signature when the focus is not on it.  So you'd see the preview, click on it, get the code, edit it, click away, and see how it looks.  This could be done in more or less the current amount of space.

> * Is there any markup we strip or disallow from signatures?  I can't think of
> anything off the top of my head or even why we would want to disallow it other
> than it means the user can insert arbitrary markup into the composer. 
> Javascript is disabled, so no worries there, but is it possible they'll break
> something in the composer if we don't?  (For example, could they break the
> composer w/ embedded CSS?)

Yes, the user could screw things up if they tried.  (Put an absolutely positioned div to cover the whole composer, for example.)  But I don't think they'll do that by accident, and I don't think it's our job to protect malicious users from themselves.

Jim, any thoughts on the above?

(In reply to comment #8)
> The existing box would still be there.  This would just be a way of adding
> WYSISYG editing without adding a whole new editor.

Yeah, I can see that.  I would call that a separate feature, deserving of a separate patch and ticket.  Let's get the basics wired into place and then look for other means.

> > * Auto-insert whitespace.
> 
> Should we do this for HTML .signature files as well?  If we do, we should
> probably careful and not replace "<br />\n" with "<br /><br />".

If you're smart enough to roll your own .signature file, it's for other software, and I don't know how they handle this.  I'm guessing they don't auto-insert whitespace, so I guess we shouldn't either.

It may be we just flip a coin and wait to see if people complain.  I'm really not sure what the convention is here.

> One way to do this would be to replace the text box with a webview to preview
> the signature when the focus is not on it.

That would be ... cool.

> Yes, the user could screw things up if they tried.  (Put an absolutely
> positioned div to cover the whole composer, for example.)  But I don't think
> they'll do that by accident, and I don't think it's our job to protect
> malicious users from themselves.

I'm not worried about that, more about someone adding some innocuous CSS and it blowing up the composer or the viewer.  Let me put it this way: if a user can put their signature in a <DIV ID="signature"> and screw us up, we need to make some changes to our CSS.

Also, do we need to deal with unclosed blocks, etc.?  I forget if your original patch handles that.

I've mocked up the preview/editor swapper.  The basic idea works, but triggering the change on focus out doesn't.  Simply clicking outside of the text box isn't enough to make the textview lose focus -- you need to click on another element that can accept focus.  But generally that means activating that other control, which is not what we want.

It may be possible to set up a more complicated click-tracking system that can detect when the user clicks outside of the textview, but I worry that this would be fragile.  We could add a "Preview" button to the editing widget that would flip it back to the webview.  Or we could use Gtk.StackSwitcher to make it explicit and give up on the focus triggering completely.

Your thoughts?

I agree, tracking clicks could be fragile.  I'm worried about Gtk.StackSwitcher because its buttons are pretty large (it's what we use for the different views in California and it's chewing up a lot of toolbar real estate).

If a "Preview" button is easy, I would say use that.  You could even use it to trigger a Gtk.Stack and get some animated effects.  I'm thinking the button would change to "Edit" (or something like that) when in Preview mode.

Created attachment 295346 [details] [review]
Add UI for previewing HTML signatures

This patch is to be applied on top of the previous one.

This adds a preview for the signature.  The editor and previewer are in 
a Gtk.Stack, and a Gtk.StackSwitcher lets you move between them.  It's 
not as slick as the automatic switch, but it should be more robust.

One problem with this is that the StackSwitcher is in the vertical 
orientation, and the Ambiance theme in 14.04 is a bit broken with this 
choice.  It looks fine in Adwaita, though, so I'm happy to blame this on 
Ubuntu and move on.

When your editing a signature in the UI, it'll preserve the whitespace 
whether or not HTML is detected.  (For a .signature file, it only does 
that if its not.)  This requires a change to how we preserve whitespace, 
as described in the commit message.

I haven't added any documentation about this yet.  What are your 
thoughts about tooltips, labels, etc?

Review of attachment 295346 [details] [review]:

Wow!  That came together a lot more easily than I thought it would.  Good work.  What I'm seeing now is mostly cosmetic:

* You probably know I'm going to say something about the buttons.  They're too bug and they're packed too close to the edit/preview window.  My thought on that is to (a) pad the two by 4px and (b) use icons instead of text for the GtkStackSwitcher.  I believe you can do that by setting the "icon-name" child property on each page.  I'm not sure if there's a way to set a tooltip on a stack switcher, but that would be nice too.  Basically, anything to reduce the size of the buttons is welcome here.

* The text for the checkbox should be something like "Sign emails (HTML allowed):"

* This isn't your doing but since you're touching the code, please change on_use_signature_changed() in add-edit-page.vala to simply be:

signature_box.sensitive = check_use_email_signature.active

Or, if you're feeling adventurous, bind the properties.

* In smart_escape(), since text is nullable, res should be nullable as well.  If null text simply returns an empty string, I would add a fast-return to the top of the method to avoid dealing with null later in the method.

* There's no real documentation in the help docs about signatures.  My feeling is that that a full explanation could be in "Accounts" with a note in "Write a message" to look there for more information about signatures.  But that's enough work to justify a separate ticket: bug #743562

This looks close.  It looks like the big problems are out of the way, just some fine detail work.

Created attachment 295499 [details] [review]
Allow HTML signatures

This should handle everything except the docs.  I moved the 
StackSwitcher back to the horizontal orientation, so it looks nice under 
Ambiance, while still taking up less space than it used to.

Perhaps we should align the StackSwitcher with the top of the 
editor/previewer?  Having it aligned with the middle can look a bit 
weird.

Also, I chose the icons based on how they looked, not based on any semantic meaning, which is probably a very bad idea.  But I didn't see any good icons to represent the actions of editing or previewing.  Any suggestions?

Yeah, let's align the switcher with the top of the edit box.  I wish we could make it vertical, but if it looks that bad under Ambiance, we can live with it until Ubuntu gets their theme fixed.

As far as icons, text-editor-symbolic is fine here.  Another possibility is the icon we use in the main Accounts page, edit-symbolic.  But since the signature is a kind of document, what you have seems fine to me.

As for preview...unfortunately, I don't have a good suggestion.  The print preview icons I looked at don't work at all.  Let's just stick with what you have for now.

I say with the alignment change this is ready to go.  Anything else before committing?

Created attachment 295613 [details] [review]
Allow HTML signatures

I got the buttons top-aligned with a Gtk.Alignment.  Is that the 
recommended solution, or is there a way to do it within the HBox?

Other than that, I think this is ready to go.

Review of attachment 295613 [details] [review]:

Actually, the way to do it is to set switcher.valign = Gtk.Align.Start.  Basically, if you ever find yourself reaching for Gtk.Alignment, you probably can use v/halign, v/hexpand, and margin properties for the same effect.

Make that change and commit!

I thought there was a better way, but my Google-fu was failing.  Thanks.

Attachment 295613 [details] pushed as c87bbea - Allow HTML signatures