Comment 129 for bug 357864

Revision history for this message
In , Josiah-l (josiah-l) wrote :

Comment on attachment 8553148
Possible patch

Review of attachment 8553148:
-----------------------------------------------------------------

Sorry for the delayed review!

UI-Review:
------------
Minor Nits:
- "Customize From Address" is confusing. “Enter a custom address” perhaps?
- If you choose a custom address (which focuses the field), and then you deselect the field, and click it again, we should show the drop down (don't keep it editable).

Significant Problems:
- What if users try to use it as a separate address and they don’t actually have the account added to TB. Any replies to it will never show up.
  * (I'm thinking of naive users who think they can enter whatever address they "own" here without bothering to add the actual account, and expect us to add it automatically or something.
- What server are we actually sending this message through? We should probably have some capability to choose which one.
- Is this going to encourage spam sending? I can send a message as “<email address hidden>” now. I understand the capability was there already, but now it’s going to be even easier to do. This feature could be seriously abused.

Conclusion:
Is this feature too accessible? Should we hide this ability somehow? I can definitely see its value, but perhaps we need a pref for it.
I like the drop down UI though, just needs an option to pick the server.

Review:
------------

::: mail/components/compose/content/messengercompose.xul
@@ +171,5 @@
>
> <command id="cmd_convertCloud" oncommand="convertSelectedToCloudAttachment(event.target.cloudProvider); event.stopPropagation();"/>
> <command id="cmd_convertAttachment" oncommand="goDoCommand('cmd_convertAttachment')"/>
> <command id="cmd_cancelUpload" oncommand="goDoCommand('cmd_cancelUpload')"/>
> + <command id="cmd_customiseFromAddress" oncommand="MakeFromFieldEditable();"

*customize please. :)

@@ +172,5 @@
> <command id="cmd_convertCloud" oncommand="convertSelectedToCloudAttachment(event.target.cloudProvider); event.stopPropagation();"/>
> <command id="cmd_convertAttachment" oncommand="goDoCommand('cmd_convertAttachment')"/>
> <command id="cmd_cancelUpload" oncommand="goDoCommand('cmd_cancelUpload')"/>
> + <command id="cmd_customiseFromAddress" oncommand="MakeFromFieldEditable();"
> + label="&customiseFromAddress.label;"/>

Ditto.

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +302,5 @@
>
> <!-- Title for the address picker panel -->
> <!ENTITY addressesSidebarTitle.label "Contacts">
>
> +<!-- Identity popup customise menuitem -->

Ditto.

@@ +303,5 @@
> <!-- Title for the address picker panel -->
> <!ENTITY addressesSidebarTitle.label "Contacts">
>
> +<!-- Identity popup customise menuitem -->
> +<!ENTITY customiseFromAddress.label "Customize From Address">

Ditto.

::: mailnews/compose/src/nsMsgCompose.cpp
@@ +1066,5 @@
> + nsCString sender;
> + MakeMimeAddress(NS_ConvertUTF16toUTF8(fullName), email, sender);
> + m_compFields->SetFrom(sender.IsEmpty() ? email.get() : sender.get());
> + }
> +

All of this is unnecessary now because of your backend changes landing.