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

2.x: Backport feature detailed results 5 to 2x #449

Merged
merged 26 commits into from
Jan 5, 2025

Conversation

dodmi
Copy link
Contributor

@dodmi dodmi commented May 30, 2024

I'm creating a pull request for my backport of displaying detailed results. So you can see the code matching the screenshots.

I'm pretty happy with the output, now. But I'm unhappy, how I solved the GUI part in chrome/content/dkim.js.
I don't really understand your code in bindings.xml (or custom XUL elements in general). So my implementation is pretty ugly and probably needs some improvement.
The clean way would probably be extending bindings.xml and not manipulating DOM objects directly...

@dodmi dodmi force-pushed the backport-feature-detailed-results-5-2x branch from 1620476 to f77bd62 Compare June 3, 2024 09:59
@lieser
Copy link
Owner

lieser commented Jul 25, 2024

Thanks for back porting this work in progress. I will try to find time this weekend to continue the work on this for 5.x, and also more closely look at your back port.

@dodmi
Copy link
Contributor Author

dodmi commented Jul 27, 2024

Feel free to use parts of my code or let me know if I should change/improve something.
I thought making a pull request helps you more than my screenshots...

I'm pretty happy, but I have no solution for cut tooltips. But this isn't a problem for one or two signatures, which is the usual amount, as I can easily see now :)

@dodmi dodmi changed the title Backport feature detailed results 5 to 2x 2.x: Backport feature detailed results 5 to 2x Dec 24, 2024
Copy link
Owner

@lieser lieser left a comment

Choose a reason for hiding this comment

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

Did not do a detailed review, but looks mostly good to me.

modules/dkimVerifier.jsm.js Outdated Show resolved Hide resolved
@lieser lieser added this to the 2.3.0 milestone Jan 1, 2025
@lieser lieser added the enhancement Improvements or new features label Jan 1, 2025
Copy link
Owner

@lieser lieser left a comment

Choose a reason for hiding this comment

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

Again didn't do a detailed review, but LGTM now overall.

Should I merge this now, or do you plan to make further changes to it as part of this PR?

@dodmi
Copy link
Contributor Author

dodmi commented Jan 5, 2025

I don't plan to change anything at the moment unless I broke something while merging the other commits.
But the only thing, which shared code was the PR to prefer ED25519 over RSA, which is still working fine.

So please merge the PR, this will make further changes easier.

@lieser lieser merged commit f66afba into lieser:2.x Jan 5, 2025
2 checks passed
@dodmi dodmi deleted the backport-feature-detailed-results-5-2x branch January 6, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements or new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants