[Kde-pim] Review Request: suspicious QString::arg() substitution in conflict resolution

[Jonathan Marten]

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

Review request for KDEPIM-Libraries.


Summary
-------

In HtmlDifferencesReporter::addProperty(), three or two arguments are substituted into a format string by repeated applications of arg().  This can cause problems, though, if an earlier substituted string contains a "%1" or "%2"; subsequent substitutions will not be made in the intended place.

The symptom of this happening is the conflict resolution box in KMail (see also bug 281085).  If the "left" message contains the string "%1" or "%2", which can happen very easily with a HTML mail containing an encoded URL or data, the "right" Data column just shows "%3".  Scrolling down will show the "right" message substituted at a random place in the "left".

Multiple strings such as this should be substituted using the multi-argument form of arg().

The patch also moves the compareItems() to before the "Data" (payload) display, so that the more relevant information (the actual conflicting data or attributes) are shown without having to scroll down to the bottom.


Diffs
-----

  akonadi/conflicthandling/conflictresolvedialog.cpp 3428b6c 

Diff: http://git.reviewboard.kde.org/r/102500/diff


Testing
-------

Built kdepimlibs and kdepim with these changes, observed correct display and operation of conflict resolution dialogue.


Thanks,

Jonathan

_______________________________________________
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/