Last Comment Bug 294140 - email autocomplete always adds "@localdomain" to addresses
: email autocomplete always adds "@localdomain" to addresses
Status: VERIFIED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey1.0alpha
Assigned To: Ian Neal
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-14 03:14 PDT by Ian Neal
Modified: 2007-10-13 15:24 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v0.1 (17.35 KB, patch)
2005-05-14 05:24 PDT, Ian Neal
mnyromyr: review-
Details | Diff | Splinter Review
Updated Patch v0.1a (18.97 KB, patch)
2005-05-16 17:05 PDT, Ian Neal
mnyromyr: review-
Details | Diff | Splinter Review
Revised Patch v0.1b (Checked in) (19.50 KB, patch)
2005-05-18 14:57 PDT, Ian Neal
mnyromyr: review+
mozilla: superreview+
asa: approval1.8b4+
Details | Diff | Splinter Review
Fix typo patch (Checked in) (1.43 KB, patch)
2005-07-14 02:44 PDT, Ian Neal
kairo: review+
benjamin: approval1.8b4+
Details | Diff | Splinter Review
fix thunderbird bustage (Checked in) (3.71 KB, patch)
2005-07-15 11:40 PDT, Scott MacGregor
mozilla: superreview+
Details | Diff | Splinter Review

Description Ian Neal 2005-05-14 03:14:50 PDT
This does the mailnews equivalents of the TB parts of bug 93453 and bug 294107
Comment 1 Ian Neal 2005-05-14 05:24:49 PDT
Created attachment 183576 [details] [diff] [review]
Patch v0.1

This patch:
* Adds the JS to make use of the new prefs introduced in bug 93453 and altered
in bug 294107
* Adds pref/account settings for the new prefs above
* Updates help to reflect the new pref/account settings
Comment 2 Frank Wein [:mcsmurf] 2005-05-16 06:04:43 PDT
BTW: Neil was thinking of another method to solve this problem, maybe you should
talk to him.
Comment 3 Karsten Düsterloh 2005-05-16 11:23:52 PDT
Comment on attachment 183576 [details] [diff] [review]
Patch v0.1

>Index: mailnews/base/prefs/resources/locale/en-US/am-addressing.dtd
>===================================================================
>+<!ENTITY autocompleteToMyDomain.label     "Automatically complete addresses with my domain">

Hm, this sounds somewhat ambiguous. Is my domain used as the completion or are
addresses with my domain automatically completed and, if so, to what? (The
former, I know.)
How about "Automatically append my domain to addresses" or something (better)
like that?

>Index: mailnews/compose/resources/content/MsgComposeCommands.js
>===================================================================
> function setDomainName()
> {
>-  var emailAddr = gCurrentIdentity.email;
>-  var start = emailAddr.lastIndexOf("@");
>-  gAutocompleteSession.defaultDomain = emailAddr.slice(start + 1, emailAddr.length);
>+  if (gCurrentIdentity.autocompleteToMyDomain)
>+  {
>+    var emailAddr = gCurrentIdentity.email;
>+    var start = emailAddr.lastIndexOf("@");
>+    gAutocompleteSession.defaultDomain = emailAddr.slice(start + 1, emailAddr.length);
>+  }
> }

This has issues with recycled compose windows (thus minussing).
As soon as you stumble upon an account with autocompletion turned on, that
settings sticks with the compose window. Any subsequent compose window with
autocompletion turned _off_ will still use the last autocompletion domain!
(And the 2nd argument to String.slice is not needed here, btw).

>+          if (sPrefs.getBoolPref("mail.autoComplete.highlightNonMatches"))
>+            document.getElementById('addressCol2#1').highlightNonMatches = true;
>+

This is pretty useless without any supporting styles like those used by TB, eg.
<http://lxr.mozilla.org/mozilla/source/mail/themes/pinstripe/mail/compose/messe
ngercompose.css#407>.
Comment 4 Ian Neal 2005-05-16 17:05:41 PDT
Created attachment 183777 [details] [diff] [review]
Updated Patch v0.1a

Changes since v0.1:
* Updated account pref label as per review
* Empties default domain when autocompleteToMyDomain is false
* Add css for highlight pref
Comment 5 Karsten Düsterloh 2005-05-18 10:55:45 PDT
Comment on attachment 183777 [details] [diff] [review]
Updated Patch v0.1a

>Index: mailnews/compose/resources/content/MsgComposeCommands.js
>===================================================================
>+function CheckValidEmailAddress(to, cc, bcc)

Since most of this file is violating the Mozilla naming guidelines, these
function arguments are in fact following the prevalent style (but I'm not quite
happy with that)...

Anyway, I found another issue:
If you have @local turned off and enter an address part that is matched by only
2 addressbook entries, the address popup isn't shown, because it has the
minResultsForPopup property of the respective textbox set to 3...
This should be 2 in this case (only), probably in setupAutocomplete. (eg.
http://lxr.mozilla.org/mozilla/source/mailnews/compose/resources/content/MsgCom
poseCommands.js#1017 for LDAP)
Comment 6 Ian Neal 2005-05-18 14:57:21 PDT
Created attachment 183953 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Changes since v0.1a: (as per reviewer's request)
* Added a reduction to minResultsForPopup for when autocompleteToMyDomain is
off
* Changed arguments to aTo, aCC and aBCC for CheckValidEmailAddress function
Comment 7 Karsten Düsterloh 2005-05-18 16:19:57 PDT
Comment on attachment 183953 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

>Index: mailnews/compose/resources/content/MsgComposeCommands.js
>===================================================================
>+      // When autocompleteToMyDomain is off there is no default entry with the domain
>+      // appended so reduce the minimum results for a popup to 2 in this case.

I feel a certain lack of commas here, but that may just be me. ;-)
Comment 8 Ian Neal 2005-05-29 15:35:33 PDT
(In reply to comment #7)
> (From update of attachment 183953 [details] [diff] [review] [edit])
> >Index: mailnews/compose/resources/content/MsgComposeCommands.js
> >===================================================================
> >+      // When autocompleteToMyDomain is off there is no default entry with
the domain
> >+      // appended so reduce the minimum results for a popup to 2 in this case.
> 
> I feel a certain lack of commas here, but that may just be me. ;-)
> 
I will add commas before checkin :-)
Comment 9 Ian Neal 2005-07-08 16:28:43 PDT
Comment on attachment 183953 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Rqeuesting sr from David as he wrote the patch for TB and should know the code.
Comment 10 Ian Neal 2005-07-09 05:06:42 PDT
Comment on attachment 183953 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Requesting approval for fairly low risk, suite-only patch which has been in TB
for a while now.
Comment 11 Ian Neal 2005-07-13 14:43:11 PDT
Comment on attachment 183953 [details] [diff] [review]
Revised Patch v0.1b (Checked in)

Checking in
extensions/help/resources/locale/en-US/mail_help.xhtml;
new revision: 1.61; previous revision: 1.60
mailnews/addrbook/prefs/resources/content/pref-addressing.xul;
new revision: 1.55; previous revision: 1.54
mailnews/addrbook/prefs/resources/locale/en-US/pref-addressing.dtd;
new revision: 1.19; previous revision: 1.18
mailnews/base/prefs/resources/content/am-addressingOverlay.xul;
new revision: 1.5; previous revision: 1.4
mailnews/base/prefs/resources/locale/en-US/am-addressing.dtd;
new revision: 1.8; previous revision: 1.7
mailnews/compose/resources/content/MsgComposeCommands.js;
new revision: 1.367; previous revision: 1.366
mailnews/compose/resources/locale/en-US/composeMsgs.properties;
new revision: 1.80; previous revision: 1.79
themes/classic/messenger/addressingWidget.css;
new revision: 1.5; previous revision: 1.4
themes/modern/messenger/addressingWidget.css;
new revision: 1.6; previous revision: 1.5
done
Comment 12 Serge Gautherie (:sgautherie) 2005-07-13 18:21:05 PDT
(In reply to comment #11)
> (From update of attachment 183953 [details] [diff] [review] [edit])
> extensions/help/resources/locale/en-US/mail_help.xhtml;
> new revision: 1.61; previous revision: 1.60

nit:
{{
Select this if you want to Mail &amp; Newsgroups to automatically [...]
[...]       If you want Mail &amp; Newsgroups to highlight [...]
}}
Isn't there a 'to' to remove/add !?
Comment 13 Ian Neal 2005-07-14 02:44:10 PDT
Created attachment 189282 [details] [diff] [review]
Fix typo patch (Checked in)

Corrects typo found in comment above
Comment 14 Ian Neal 2005-07-14 02:51:37 PDT
Comment on attachment 189282 [details] [diff] [review]
Fix typo patch (Checked in)

Requesting a= for very simple typo fix
Comment 15 Ian Neal 2005-07-15 05:51:25 PDT
Comment on attachment 189282 [details] [diff] [review]
Fix typo patch (Checked in)

Checking in mail_help.xhtml;
new revision: 1.62; previous revision: 1.61
done
Comment 16 Scott MacGregor 2005-07-15 11:27:58 PDT
I think this broke the Compositions &  Addressing panel in Thunderbird's account
Wizard. I get a entity error in today's build.  
Comment 17 Scott MacGregor 2005-07-15 11:40:35 PDT
Created attachment 189448 [details] [diff] [review]
fix thunderbird bustage (Checked in)

Fixes the thunderbird bustage by not including the new piece of UI in
thunderbird.   We're not interested in exposing this setting via the account
settings UI for thunderbird anyway so this works better than fixing the string
entity.
Comment 18 Scott MacGregor 2005-07-15 13:06:27 PDT
Comment on attachment 189448 [details] [diff] [review]
fix thunderbird bustage (Checked in)

I checked in the fix for the thunderbird bustage.
Comment 19 Stephen Donner [:stephend] 2006-01-04 10:15:01 PST
Verified FIXED on trunk using both SeaMonkey 1.5a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060104 Mozilla/1.0 and Thunderbird version 1.6a1 (20060103).

I verified that on Thunderbird, we don't add the user's domain by default, and that there is no such UI exposed to enable it.

Note You need to log in before you can comment on or make changes to this bug.