headers
Frank Reininghaus | 23 May 2013 22:55

Review Request 110625: Make it possible to disable KAbstractFileItemActionPlugins by default

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

Review request for kdelibs.
By Frank Reininghaus.

Description

Currently, all KAbstractFileItemActionPlugins which are installed are embedded into file management-related context menus unless the user explicitly disables them in the settings. This becomes problematic if the plugin's actions() method executes code that is not guaranteed to return very fast - the user will notice that the host application freezes without even knowing that one of the plugins is the culprit. In a perfect world, all code executed by plugins would be perfect and the problem would not exist at all. Unfortunately, the world is not perfect, and there are plugins which do things that succeed most of the time, but cause trouble sometimes: https://bugs.kde.org/show_bug.cgi?id=314575 It seems that what Activities is trying to achieve cannot easily be solved using a safer approach which is equally user-friendly for Activities users. On the other hand, I don't think that people who do not use the plugin at all should suffer from the freezes caused by the plugin, so I thought that we could give developers of plugins which execute some "fragile" code the chance to declare that only users who enable the plugin explicitly in the settings should see the plugin's actions in their context menu. For further information, please see https://bugs.kde.org/show_bug.cgi?id=314575. Unfortunately, the original author of KAbstractFileItemActionPlugin chose to not add a d-pointer, so I had to use a hack to make it work. I have patches that make this work: kactivities (includes another change that fixes a build failure for me): http://paste.kde.org/749834/ kde-baseapps: http://paste.kde.org/749846/

Testing

Deleted kservicemenurc -> Activities not shown in Konqueror/Dolphin context menus, and the plugin is shown as disabled in the settings. Enabled the plugin in the settings dialog of either Konqueror or Dolphin -> plugin is shown, and disabling it in the dialog hides it again.

Diffs

  • kio/kio/kabstractfileitemactionplugin.h (6af7396)
  • kio/kio/kabstractfileitemactionplugin.cpp (07f15f6)

View Diff
Permalink | Reply | 0 comments |
headers
Shaheed Haque | 20 May 2013 22:26
Favicon

KTrader and "search paths"

HI,

Over in Kate's mailing list (http://lists.kde.org/?l=kwrite-devel&m=136804130419019&w=2) there is a discussion about how to support the discovery of plugins written in Python. Currently, this is done in a very Python-centric way using KStandardDirs to find the plugins, and then docstrings within the .py files to load (for example) the description of the module. One problem that has emerged from this approach is that i18n of the docstring-based description strings is problematic, and one possible option to deal with this is to use .desktop files in the usual way.

First, note that the plugins written in Python don't plugin to Kate in the normal sense of a KDE C++ plugin, but they themselves plugin into a "hosting" plugin called Pate which *is* a normal KDE C++ plugin with a .desktop file and all.

Second these Python plugins are intended to encourage user-hacking by allowing a "search path" concept. So, if the system has the foo Python plugin in these three places:

$HOME/.kde/share/apps/kate/pate          <<< my in-development plugin versions
/usr/local/share/apps/kate/pate                <<< my locally installed plugins
/usr/share/kde4/apps/kate/pate                <<< distro default installation

then Pate will (a) load the user's version but also (b) show the user than the other two versions exist, but have been overridden. The idea being that user's get the notion that s/he can simply copy a system plugin to a directory under $HOME and hack it.

I believe if I combined the current manual walking of the KStandardDirs with KService, I could make this work, I wondered if KTrader might be used to simplify things a bit. I've done some experiments with ktraderclient, and think the following issues would have to be overcome:

1. Is there a location under $HOME where KTrader will look for desktop files?
2. Does KTrader support a search path, and if it does, can it return the non-first hits?
3. The DesktopEntryPath returned by KTrader is not a fully qualified path (the docs for KService::desktopEntryPath() and KService::entryPath() suggest this might be possible, but I cannot fathom how to trigger this behaviour via KTrader)

Any ideas welcome!

Thanks, Shaheed

Permalink | Reply | 0 comments |
headers
Friedrich W. H. Kossebau | 20 May 2013 20:34
Picon
Favicon

Review Request 110563: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

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

Review request for Build System and kdelibs.
By Friedrich W. H. Kossebau.

Description

Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple times into the same process (with Bsymbolic-functions flag)" on kde-core-devel ( http://lists.kde.org/?t=136829863100001&r=1&w=2 ) symbols from QtUitools.a are not hidden by default in Qt4 and thus will be added to the public symbols of the module/shared lib they are linked to. And thus can appear multiple times in the same process, resulting in symbol clashes and leading to problems at least with the Bsymbolic-functions flag or when being possibly incompatible versions. Attached patch sees to solve that problem, by adding a macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags depending on the platform/linker used. Only issue is that instead of some variable I had to use "QtUiTools.a" as I found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case another platform needs another name/prefix here? Patch is against 4.10 branch, so I hope to get this in 4.10.4 http://lxr.kde.org/search?v=4.10-branch&filestring=&string=QT_QTUITOOLS_LIBRARY shows that there are some more places where the symbols need hiding, but I first want feedback on the proposed approach.

Diffs

  • cmake/modules/FindKDE4Internal.cmake (cb63285)
  • cmake/modules/KDE4Macros.cmake (3db4e24)
  • kjsembed/kjsembed/CMakeLists.txt (d70f260)
  • kross/modules/CMakeLists.txt (d245fd8)
  • kross/qts/CMakeLists.txt (d8cb4a5)
  • plasma/CMakeLists.txt (674550d)

View Diff
Permalink | Reply | 4 comments |
headers
Ian Monroe | 20 May 2013 04:05
Picon
Gravatar

Review Request 110529: more error handling in KIdleTime

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

Review request for kdelibs.
By Ian Monroe.

Description

more error handling in KIdleTime Segfaults could result in some rare circumstances

Diffs

  • tier1/kidletime/src/xsyncbasedpoller.cpp (e5f5328ae66d44e9224582ad759207bc42333d80)
  • tier1/kidletime/src/kidletime.cpp (fe18ee5d5c4525086a56d99e76e8ea0a4a92ce08)

View Diff
Permalink | Reply | 4 comments |
headers
Michael Pyne | 19 May 2013 00:08
Picon
Favicon

Re: Review Request 110367: Add JoinTheGame menu entry

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

This caused an unhandled switch warning in kdoctools/genshortcutents.cpp, which can be trivially fixed with: diff --git a/kdoctools/genshortcutents.cpp b/kdoctools/genshortcutents.cpp index b35b5a4..92bdea5 100644 --- a/kdoctools/genshortcutents.cpp +++ b/kdoctools/genshortcutents.cpp <at> <at> -300,6 +300,9 <at> <at> static QString entityForAccel( KStandardShortcut::StandardShortcut accel ) case KStandardShortcut::AboutKDE: markup += "AboutKDE"; break; + case KStandardShortcut::JoinTheGame: + markup += "JoinTheGame"; + break; case KStandardShortcut::StandardShortcutCount: break; } however I'm not sure if there is anything else needed for the doc tools. This additional patch allowed kdelibs docs to build just fine so I don't think there are additional XML schemas to modify, however it might be prudent to check. ----------- As a separate point though, I must admit to being nervous about the idea of integrating this into *every* KDE-using app's UI, including applications that have no affiliation with KDE whatsoever other than using kdelibs. How would we as devs/GNU-users react if the default bash prompt contained a notice on how to fund the FSF? Additionally even if we assume that users of KDE software wish to fund contributor meetings, necessary infrastructure, and the future security of KDE software, it doesn't necessarily follow that they wish to e.g. raise awareness for Free Culture. In any event I would recommend avoiding "baking in" a donation amount or method in the U/I, it may change in the future. ------------ I don't want to splash cold water on the idea because I think this kind of promo activity can be very useful, but I don't think this is the right format. I would recommend a separate app or Plasmoid that can be run with a way to link it from the Plasma desktop (which *does* belong to KDE), preferably not quite as obtrusively. We're currently riding a relative wave of positive karma with FOSS users for the first time in years and I don't want to see KDE exposed to the same reaction that we've seen with Ubuntu's Amazon gem or earlier with KDE 4.0. I've grown fond of not having flamewars about cashews or JRTs and I wish to see that continue if possible. :)
kdeui/dialogs/kjointhegamedialog_p.cpp (Diff revision 3) 76
"<li>Regular first hand reports about KDE's activities do so</li></ul> "
"activities do so" is wrong, I'm not sure what exactly is meant though.

- Michael


On May 9th, 2013, 11:05 p.m. UTC, Albert Astals Cid wrote:

Review request for kdelibs.
By Albert Astals Cid.

Updated May 9, 2013, 11:05 p.m.

Description

Patch by Pau ages ago, Lydia wants to push for it again. Note that i still find it ugly, but i'm not the kdeui maintainer, so i'll leave it to someone else to discuss. Not sure if the StandardAction enum addition should be at the end in case someone is storing those enums in a file or something.

Testing

Ran okular, saw the new menu entry, it did stuff. Ran ksnapshot (that doesn't use xmlgui), saw the new menu entry, it did the same stuff.

Diffs

  • kdecore/doc/README.kiosk (b95002d)
  • kdeui/CMakeLists.txt (b439e04)
  • kdeui/actions/kstandardaction.h (3064301)
  • kdeui/actions/kstandardaction.cpp (7de0c6f)
  • kdeui/actions/kstandardaction_p.h (b8f8df1)
  • kdeui/dialogs/jointhegame.png (PRE-CREATION)
  • kdeui/dialogs/kaboutkdedialog_p.cpp (b9728bf)
  • kdeui/dialogs/kjointhegamedialog_p.h (PRE-CREATION)
  • kdeui/dialogs/kjointhegamedialog_p.cpp (PRE-CREATION)
  • kdeui/shortcuts/kstandardshortcut.h (b31e45c)
  • kdeui/shortcuts/kstandardshortcut.cpp (669f750)
  • kdeui/widgets/khelpmenu.h (3389068)
  • kdeui/widgets/khelpmenu.cpp (f547c46)
  • kdeui/xmlgui/ui_standards.rc (a0f5bed)

View Diff
Permalink | Reply | 0 comments |
headers
Sergio Luis Martins | 18 May 2013 21:43
Picon
Gravatar

Review Request 110514: Me more precise about the error message in case KSaveFile fails.

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

Review request for kdelibs and David Faure.
By Sergio Luis Martins.

Description

"Unable to open temporary file" isn't very informative.

Testing

yep

Diffs

  • kdecore/io/ksavefile.cpp (6cdd41e)

View Diff
Permalink | Reply | 3 comments |
headers
Albert Astals Cid | 18 May 2013 15:33
Picon
Favicon
Gravatar

Re: Review Request 110367: Add JoinTheGame menu entry

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

ShipIt from anyone?

- Albert


On May 9th, 2013, 11:05 p.m. UTC, Albert Astals Cid wrote:

Review request for kdelibs.
By Albert Astals Cid.

Updated May 9, 2013, 11:05 p.m.

Description

Patch by Pau ages ago, Lydia wants to push for it again. Note that i still find it ugly, but i'm not the kdeui maintainer, so i'll leave it to someone else to discuss. Not sure if the StandardAction enum addition should be at the end in case someone is storing those enums in a file or something.

Testing

Ran okular, saw the new menu entry, it did stuff. Ran ksnapshot (that doesn't use xmlgui), saw the new menu entry, it did the same stuff.

Diffs

  • kdecore/doc/README.kiosk (b95002d)
  • kdeui/CMakeLists.txt (b439e04)
  • kdeui/actions/kstandardaction.h (3064301)
  • kdeui/actions/kstandardaction.cpp (7de0c6f)
  • kdeui/actions/kstandardaction_p.h (b8f8df1)
  • kdeui/dialogs/jointhegame.png (PRE-CREATION)
  • kdeui/dialogs/kaboutkdedialog_p.cpp (b9728bf)
  • kdeui/dialogs/kjointhegamedialog_p.h (PRE-CREATION)
  • kdeui/dialogs/kjointhegamedialog_p.cpp (PRE-CREATION)
  • kdeui/shortcuts/kstandardshortcut.h (b31e45c)
  • kdeui/shortcuts/kstandardshortcut.cpp (669f750)
  • kdeui/widgets/khelpmenu.h (3389068)
  • kdeui/widgets/khelpmenu.cpp (f547c46)
  • kdeui/xmlgui/ui_standards.rc (a0f5bed)

View Diff
Permalink | Reply | 1 comment |
headers
Marco Martin | 17 May 2013 14:11
Picon
Gravatar

Review Request 110482: Move KStatusNotifierItem in KNotifications

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

Review request for kdelibs.
By Marco Martin.

Description

This implemets a step in the kdeui crumble epic. moves the classes kstatusnotifieritem and knotificationsrestrictions in the knotifiactions library. The patch works, but there are still several issues: * porting from kdebug to qdebug loses the area number * adds some link libraries: the classes add ki18n, kwidgets and KWidgetsAddons * the test adds ki18n kde4support kdecore * the KActionCollection becomes a qhash of actions: how should be kactioncollections ported? I guess it should use the qt translation system, and redo the quit dialog to not usekstandardgui at all?

Diffs

  • kdeui/CMakeLists.txt (cfa29ef)
  • kdeui/tests/CMakeLists.txt (cd055d5)
  • staging/knotifications/src/CMakeLists.txt (266b67c)
  • staging/knotifications/src/knotificationrestrictions.cpp (a396fd6)
  • staging/knotifications/src/kstatusnotifieritem.h (be21882)
  • staging/knotifications/src/kstatusnotifieritem.cpp (37abe7e)
  • staging/knotifications/src/kstatusnotifieritemdbus_p.cpp (6c9e1da)
  • staging/knotifications/src/kstatusnotifieritemprivate_p.h (32e7906)
  • staging/knotifications/tests/CMakeLists.txt (2240a69)
  • staging/knotifications/tests/kstatusnotifieritemtest.cpp (38e85ac)

View Diff
Permalink | Reply | 11 comments |
headers
Valentin Rusu | 16 May 2013 21:07

macro_log_feature : setting max version

Hello,

This issue is triggered by kopete's OTR plugin that stopped compiling with 
LibOTR 4.0.0 because it undergone some imported structure changes.

Pali Rohar managed to modify the FindLibOTR.cmake, we now set-up a version 
range starting with 3.2.0 (initial requirement) to 4.0.0 and kopete compiles 
by excluding the offending, too recent, LibOTR.

However, the final cmake status outputs this :

-----------------------------------------------------------------------------
-- The following OPTIONAL packages could NOT be located on your system.
-- Consider installing them to enable more features from this software.
-----------------------------------------------------------------------------
   * libotr (3.2.0 or *higher*)  <http://www.cypherpunks.ca/otr>
     A library to encrypt messages with Off-the-Record encryption (versions 
3.2.0 to 4.0.0)
     Required for the Kopete otr plugin.

I think tat a more elegant output message would state :

* libotr (3.2.0 to 4.0.0)

But the macro does not have a _maxVersion parameter. How about adding it? 
Would it be OK to add such a parameter? 

--

-- 
Valentin Rusu (vrusu)
IRC: valir
Permalink | Reply | 1 comment |
headers
Dan Vrátil | 16 May 2013 20:21
Picon
Favicon
Gravatar

Review Request 110476: Call KNotification::close() when notification is closed in the applet

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

Review request for kde-workspace.
By Dan Vrátil.

Description

Call KNotification::close() when user clicks the 'X' button in the Notifications applet. This prevents leaking persistent notifications and allows applications to depend on KNotification::closed() signal.

Diffs

  • plasma/generic/applets/notifications/contents/ui/NotificationDelegate/NotificationDelegate.qml (64d9298)
  • plasma/generic/applets/notifications/contents/ui/Notifications.qml (c5be0a3)

View Diff
Permalink | Reply | 1 comment |
headers
Andre Heinecke | 16 May 2013 12:21
Picon
Favicon

Review Request 110462: Call new Instance dbus method on Windows when a KUniqueApplication is already running

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

Review request for kdelibs, kdewin, Patrick Spendrin, and Patrick von Reth.
By Andre Heinecke.

Description

Currently when a second instance of a KUniqueApplication is created KUniqueapplication on Windows just tries to bring the existing process to the front. The problem with this is that the commandline arguments are lost. Kontact parts of kdepim solved this problem in pimuniqueapplication by calling NewInstance with the correct parameters on the existing application. This review Request is about adding that code from kdepimlibs/kontactinterface/pimuniqueapplication.cpp also to KUniqueapplication on Windows.

Testing

Tested with Kleopatra on Windows.
Bugs: 258489

Diffs

  • kdeui/kernel/kuniqueapplication.cpp (777fc35)

View Diff
Permalink | Reply | 0 comments |

Gmane