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.
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/ messengercompos e.xul convertCloud" oncommand= "convertSelecte dToCloudAttachm ent(event. target. cloudProvider) ; event.stopPropa gation( );"/> convertAttachme nt" oncommand= "goDoCommand( 'cmd_convertAtt achment' )"/> cancelUpload" oncommand= "goDoCommand( 'cmd_cancelUplo ad')"/> customiseFromAd dress" oncommand= "MakeFromFieldE ditable( );"
@@ +171,5 @@
>
> <command id="cmd_
> <command id="cmd_
> <command id="cmd_
> + <command id="cmd_
*customize please. :)
@@ +172,5 @@ convertCloud" oncommand= "convertSelecte dToCloudAttachm ent(event. target. cloudProvider) ; event.stopPropa gation( );"/> convertAttachme nt" oncommand= "goDoCommand( 'cmd_convertAtt achment' )"/> cancelUpload" oncommand= "goDoCommand( 'cmd_cancelUplo ad')"/> customiseFromAd dress" oncommand= "MakeFromFieldE ditable( );" &customiseFromA ddress. label;" />
> <command id="cmd_
> <command id="cmd_
> <command id="cmd_
> + <command id="cmd_
> + label="
Ditto.
::: mail/locales/ en-US/chrome/ messenger/ messengercompos e/messengercomp ose.dtd rTitle. label "Contacts">
@@ +302,5 @@
>
> <!-- Title for the address picker panel -->
> <!ENTITY addressesSideba
>
> +<!-- Identity popup customise menuitem -->
Ditto.
@@ +303,5 @@ rTitle. label "Contacts"> dress.label "Customize From Address">
> <!-- Title for the address picker panel -->
> <!ENTITY addressesSideba
>
> +<!-- Identity popup customise menuitem -->
> +<!ENTITY customiseFromAd
Ditto.
::: mailnews/ compose/ src/nsMsgCompos e.cpp (NS_ConvertUTF1 6toUTF8( fullName) , email, sender); >SetFrom( sender. IsEmpty( ) ? email.get() : sender.get());
@@ +1066,5 @@
> + nsCString sender;
> + MakeMimeAddress
> + m_compFields-
> + }
> +
All of this is unnecessary now because of your backend changes landing.