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

[Sandro Knauß]

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


Fix it, then Ship it!





kleopatra/models/useridlistmodel.h (line 6)
<https://git.reviewboard.kde.org/r/127059/#comment63099>

    2016 already :D


- Sandro Knauß


On Feb. 18, 2016, 2:46 p.m., Andre Heinecke wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> 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
> -----
> 
>   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/