Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtual identity plugin #498

Merged
merged 22 commits into from Nov 18, 2011
Merged

Conversation

absorb-it
Copy link
Contributor

Hi Jonathan,

I cleaned the plugin code and tried to make the whole thing more useful to everybody. Please have a look and tell me what you think about it.

Thanks, regards, Rene


let mainWindow = getMail3Pane();

let virtualIdentityHook = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not going to work if the user right-clicks on a folder, hits "open" (opens a new window), and closes the first window, because then the mainWindow will be an invalid reference. What you need to do is call getMail3Pane() everywhere instead virtualIdentityHook so that it gets the most recent mail:3pane window (which is guaranteed to be a valid one).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, think I understand this. But seems like the enigmail plugin does the same thing, in https://github.com/absorb-it/GMail-Conversation-View/blob/5a2d3ba278af4fba62bebc03807c736d9fbfc935/modules/plugins/enigmail.js#L544 ? Or do I misunderstand it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the Enigmail plugin might be wrong... nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that to call getMail3Pane() .

@protz
Copy link
Collaborator

protz commented Nov 13, 2011

Hey,

This has gotten much better! I'm really happy with the changes you made and they all seem to look solid. I still need to take a more detailed look at it (I'm still at the mozcamp right now) but you seem to be on the right track. I'm going to ask hiroshi to perform a detailed review of the various parts he knows (better than me).

Thanks for the good work,

jonathan

@@ -277,6 +277,8 @@ $.TokenList = function (input, settings) {
return false;
});

token_count++;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the user adds a recipient manually, by hitting "add cc", and the filling in the details of the recipient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to detect if a recipient is the first entered recipient, or if there was any other recipient added before (by the user, by quickreply or however). the token_count seems to be used only to limit the shown input-fields, therefore I thought it's ok to misuse this counter. This 'fix' is to count the preconfigured recipients in the init_list too, not sure if this is ok.

@absorb-it
Copy link
Contributor Author

e8b8c96 : as suggested I removed the plugin-code completely to the virtualIdentity extension absorb-it/Virtual-Identity@7508872aa2cf74a11f

a3d7f09 : change discussed with Hiroshi to enable cleanup of encrypted mail-content if sending was cancelled

@@ -446,7 +446,7 @@ let enigmailHook = {

let uiFlags = nsIEnigmail.UI_INTERACTIVE;

let identity = aAddress.identity
let identity = aAddress.params.identity
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shirosaki, can you confirm that this change is valid?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change would be valid.

protz added a commit that referenced this pull request Nov 18, 2011
Major changes to the plugin architecture. New functions to interact with the composition process, general cleanups to allow for a better interaction between various addons.
@protz protz merged commit 5cd992c into thunderbird-conversations:master Nov 18, 2011
@protz
Copy link
Collaborator

protz commented Nov 18, 2011

Ok, I've merged this pull request. The code quality has improved a lot since the first pull request, many thanks to Rene and Hiroshi for taking the time to polish this :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants