[Kde-pim] Review Request 127059: Refactor UserID Certificate details

[Andre Heinecke]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127059/
-----------------------------------------------------------

(Updated Feb. 18, 2016, 2:46 p.m.)


Review request for KDEPIM.


Changes
-------

Addressed issues pointed out in review.


Bugs: 357933
    http://bugs.kde.org/show_bug.cgi?id=357933


Repository: kdepim


Description
-------

Friday evening bugfixing time. I thought I'll take an easy one with BUG #357933 but ended up rewriting the useridlistmodel in kleopatra.
The crash in the bug was introduced by a small "Fix logic" commit by laurent ( 9794c508 ) but WTF? The compiler warning fixed with that commit complained about that one statement was always true (internalID is unsigned) and the other always false. Reverting this commit fixed the crash but the Details Dialog did not show any information.

I've tried to understand what the original code meant to do and failed. I guess if code is so obscure that even Laurent introduces drive by crashes it's time to rewrite it. This kind of optimization to avoid copying information is really not needed here. The useridlistmodel is used in the certificate details dialog. It's only showing the certificate details for one key.

So I've rewritten this as a simple treemodel which I find much easier to understand and hack. (Also using Qt API for the same reason)

While testing this I've noticed that even the largest keys in my keyring immediately showed all user ID's and Certifications and thus removed the extra button to load them because if a user navigates into this dialog to look at the User ID's and Certifications the extra click is just annoying.

Fwiw: I'm planning to change this dialog some more to have a Status indication that does not requires you to have read RFC 4880 (BUG #358878 ). This new model will help with that.


Diffs (updated)
-----

  kleopatra/dialogs/certificatedetailsdialog.h 09058d9 
  kleopatra/dialogs/certificatedetailsdialog.cpp d1a799f 
  kleopatra/dialogs/certificatedetailsdialog.ui 3c98a91 
  kleopatra/models/useridlistmodel.h c63b1f3 
  kleopatra/models/useridlistmodel.cpp c4bd24c 

Diff: https://git.reviewboard.kde.org/r/127059/diff/


Testing
-------

Crashed before. Works now.


Thanks,

Andre Heinecke

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/