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

Review request for KDE Software on Mac OS X and Plasma.


Repository: plasma-workspace


Description
-------

While looking to build DrKonqi for OS X/MacPorts I came across a few issues 
that appear to be syntax/coding errors that, surprisingly, were never caught 
before given that they also caused compile failures when I used the same 
configure/build/packaging script on Linux.

The code uses methods that no longer exist in Qt 5.5+ : `QString::fromAscii()`, 
`QHeaderView::setResizeMod()` (replaced with 
`QHeaderView::setSectionResizeMode()` and `QInputDialog::getInteger()` (instead 
of `QInputDialog::getInt()`). Those are the easy and straightforward ones fixed 
by this patch.

The handling of file dialogs (in 2 locations) was more problematic. Both 
Apple's clang "6.0.0" and gcc 5.3 (on Linux) refused statements like 
`QWeakPointer<QFileDialog> dlg = new QFileDialog(parent,defname);` . My initial 
attempt was to follow the documentation for QWeakPointer, and do 
`QWeakPointer<QFileDialog> dlg = QSharedPointer<QFileDialog>(new 
QFileDialog(parent,defname));` but that caused a crash as soon as I clicked on 
the "save backtrace as" button - on both platforms.
It turned out that `QSharedPointer<QFileDialog>()` returned a valid (non-null) 
shared pointer, but after creating a QWeakPointer from it, `dlg.data()` 
returned NULL -- immediately.

I had a look around in the 5.17.0 frameworks codebase, and could find no 
relevant examples of using QWeakPointer; some expressions appear to be 
equivalent to the original DrKonqi code, in other locations QWeakPointer 
instances are indeed created "going through" a QSharedPointer. In the end I 
tested using QFileDialog (and `QFileDialog::selectedUrls()` as kate5 does, and 
that works (on OS X; haven't had the chance to test on Linux yet).

That's what the attached patch does: no QFileDialog pointer at all, just an 
instance.
This RR is intended to fuel a discussion on the subject, as suggested by Aleix; 
therefore I left the comment about the possibility of invalidating/deleting the 
file dialog e.g. through a DBus call. Is that actually a possibility, and why 
wouldn't it be in Kate? Are there no other ways to prevent it, or check for its 
occurrence?

This patch also marks drkonqi as a nongui executable, a requirement on OS X and 
which shouldn't cause any regressions elsewhere AFAIK.


Diffs
-----

  drkonqi/CMakeLists.txt deb8c40 
  drkonqi/bugzillaintegration/bugzillalib.cpp 802c5fb 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp e60fb14 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla_duplicates.cpp 
4f8f4ea 
  drkonqi/drkonqi.cpp b12c118 

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


Testing
-------

On OS X 10.9.1 and Kubuntu 14.04, both with Qt 5.5.1 and KF5 5.17.0 installed 
into /opt/local


Thanks,

René J.V. Bertin

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to