I've ported the two changes we still missed from the patch in bug 464770 and included some comments cleanup.

+ // We have to do prefill filter so we are going to launch the + // filterEditor dialog and prefill that with the emailAddress. We need to prefill the filter so we launch the filterEditor dialog prefilled with the emailAddress. + args = { filterList: folder.getEditableFilterList(msgWindow) }; args.filterName = emailAddress; args = { filterList: folder.getEditableFilterList(msgWindow), filterName: emailAddress }; - /* if the user hits ok in the filterEditor dialog we set - args.refresh=true there - we check this here in args to show filterList dialog */ + // If the user hits ok in the filterEditor dialog we set args.refresh=true + // there we check this here in args to show filterList dialog. I think that in the original comment there are two sentences: 1. If the user hits ok in the filterEditor dialog we set args.refresh=true there. 2. We check for refresh in args here to see if we need to show the filterList dialog. You seem to have conflated these two.

Comment on attachment 388065 [details] [diff] [review] port v1 >+ args = { filterList: folder.getEditableFilterList(msgWindow) }; > args.filterName = emailAddress; [I wonder why we don't just define the object directly with both properties.] >+ // If the user hits ok in the filterEditor dialog we set args.refresh=true >+ // there we check this here in args to show filterList dialog. As Philip says, this should be "there. We" (reformatting at your discretion).

(In reply to comment #1) > + // We have to do prefill filter so we are going to launch the > + // filterEditor dialog and prefill that with the emailAddress. > We need to prefill the filter so we launch the filterEditor dialog prefilled > with the emailAddress. I still think this is a bit repetitive... how about We are going to prefill the filterEditor dialog with the emailAddress.

> I still think this is a bit repetitive... how about > We are going to prefill the filterEditor dialog with the emailAddress. Or just: "Prefill the filterEditor dialog with the emailAddress."

I rewrote the comment and addressed the other review nits.

Comment on attachment 388894 [details] [diff] [review] Patch v2 >diff --git a/suite/mailnews/mailWindowOverlay.js b/suite/mailnews/mailWindowOverlay.js >+ args = { filterList: folder.getEditableFilterList(msgWindow), >+ filterName: emailAddress; }; You didn't test that, I'm sure. ;-) Just write: args = {filterList: folder.getEditableFilterList(msgWindow), filterName: emailAddress}; r=me with that.

Removed the superfluous ';' Taking over r+/sr+

+ args = { filterList: folder.getEditableFilterList(msgWindow), + filterName: emailAddress }; You didn't notice that Mnyromyr also removed the leading and trailing spaces in {}

Pushed with whitespace fixup as <http://hg.mozilla.org/comm-central/rev/c324b2a2bbc2>.