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

Adapt Thunderbird Conversations plugin to work with add-on version 4.x #203

Closed
lieser opened this issue Jun 11, 2020 · 5 comments
Closed
Milestone

Comments

@lieser
Copy link
Owner

lieser commented Jun 11, 2020

The current plugin for this add-on inside Thunderbird Conversations no longer work with the 4.x WebExtension version of the add-on.

See also thunderbird-conversations/thunderbird-conversations#1431

@lieser lieser added this to the 4.0.0 milestone Jun 11, 2020
@lieser lieser modified the milestones: 4.0.0, 4.1.0 Nov 9, 2020
@Standard8
Copy link

I'd be quite happy to collaborate on this. Can you give me an overview of how DKIM Verifier currently works?

I'm just wondering if there's something simpler that what we do now, that we might be able to head to with WebExtension APIs (that might be longer term, but let's see).

@lieser
Copy link
Owner Author

lieser commented Nov 23, 2020

Thanks for reaching out to me. Planned to contact you after I'm done with the complete migration (4.0.0 version), but seems like you there faster.

Can you give me an overview of how DKIM Verifier currently works?

Most parts are already in the new WebExtension format.

  1. Entry point for TB is the WebExtension browser.messageDisplay.onMessageDisplayed listener API, see
    browser.messageDisplay.onMessageDisplayed.addListener(async (tab, message) => {
  2. The listener checks if it should do anything at all i.e. that the message is an email (and not e.g. RSS post), and what UI elements (implemented via experiments) it should show (based on the add-ons options)
  3. The actually logic for verification starts in the verify method of the AuthVerifier see
    /**
    * Verifies the authentication of the msg.
    *
    * @param {browser.messageDisplay.MessageHeader} message
    * @return {Promise<AuthResult>}
    */
    async verify(message) {

    This is basically nearly the same as is currently already called by the plugin in Conversations: https://github.com/thunderbird-conversations/thunderbird-conversations/blob/89afc4bc5489d06f362a8f91b8b96db22630dfd1/addon/content/modules/plugins/dkimVerifier.js#L71

I thing the rest should be mostly irrelevant for the integration.


I already thought previously a little about the integration (nothing tried out so far), and here are my thoughts how the current plugin cold be adapted:

1. Getting the authentication result from the DKIM Verifier in the Conversations plugin:

At least for the DKIM Verifier, the easiest here would be to use the browser.runtime.sendMessage API that the WebExtensions provide. I already use that in one place inside the add-on and it was pretty straight forward to do.
browser.runtime.connect would also be an option. Haven't tried it so far, but looks a little more complicated.

One think to consider here is if the second step described above needs also be handled here by the DKIM Verifier, or if will get called for emails only anyway.

Only issue is see here is that the plugins logic in Conversation seems to not be ported to WebExtensions so far, so maybe not straight forward to do that there.

2. Showing the authentication result in Conversation:

I think here you can better tell if/what needs to be changed. the thinks returned by the AuthVerifier should be basically the same.

Not directly tied to adapting the current integration, but something that I think should be considered at least in the future:
The DKIM Verifier can show the favicon for e.g. PayPal, if a message from them was successfully verified. Example can be seen here:https://github.com/lieser/dkim_verifier/wiki/Display-Options#show-the-favicon-of-known-signing-domains-before-the-from-address
I think it would make sense to implement something similar for the Conversation plugin.

3. Only let the DKIM Verifier do it's starting of the verification (and showing the result) if a single message is viewed in the classic view

This is currently done here: https://github.com/thunderbird-conversations/thunderbird-conversations/blob/89afc4bc5489d06f362a8f91b8b96db22630dfd1/addon/content/modules/plugins/dkimVerifier.js#L34-L43

The problem is that with Conversation installed, there are two e-mail views, that needs to be handled differently:

(a) Mails are shown in the Conversation view.
We then need to either make sure that browser.messageDisplay.onMessageDisplayed is not called by TB (the current approach), or somehow the Conversation add-on needs to tell the DKIM Verifier that it shouldn't do anything, e.g. for all messages shown in a specific tab.

(b) A single mail is viewed in a separate tab or window.
I would assume that with Conversation installed the browser.messageDisplay.onMessageDisplayed will still be called normally in this case, so nothing really to do here.

I think this would be the hardest part.


Given the problem on the 3 step of the current approach has, we should at least briefly also consider to revert the current approach. I.e. that the DKIM Verifier looks for the Conversation add-on, and initiates the showing in Conversation if needed. Note that I haven't thought about that much, so unsure if it would solve anything, and what the consequences would be.
But even if the decide to revert some of the current logic, and the showing in Conversation will is triggered by the DKIM Verifier, I think the actually logic for showing the result in the Conversation view should stay in Conversation.

@lieser
Copy link
Owner Author

lieser commented Sep 19, 2021

Re-added support for Conversations using the new Pill message API (thunderbird-conversations/thunderbird-conversations#1690).

@lieser lieser closed this as completed Sep 19, 2021
@sha-265
Copy link

sha-265 commented Oct 5, 2023

Not working for me (or I just can't find where to find this "pill"?). I'm using TB 115.3.1, DKIM verifier 5.3.1 and Conversations 4.1.3.

Any suggestions?

@lieser
Copy link
Owner Author

lieser commented Oct 5, 2023

Please make sure you try it with an email that has a DKIM signature. E.g. by viewing the message in a new window, which should show the DKIM result in the header.

Otherwise take a look at the error log, maybe something shows up there https://github.com/lieser/dkim_verifier/wiki/Debug#view-error-and-debug-messages.

It could be that the detection of the conversation view is broken in case Conversation recently changed somthing there.

Please create a separate issue to track your problem.
I will try to take a look at it next week, but unfortunately could take a few weeks until I find time and have access to a system there I can test it.

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

No branches or pull requests

3 participants