headers
Commit Hook | 1 Feb 16:35
Picon
Favicon

Re: Review Request: Make the writeConfig() method a public slot

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

This review has been submitted with commit 827d7cb30d61fca61beab87cde2c14d64ab7d150 by Laszlo Papp to branch frameworks.

- Commit


On January 30th, 2012, 8:57 p.m., Laszlo Papp wrote:

Review request for kdelibs and David Faure.
By Laszlo Papp.

Updated Jan. 30, 2012, 8:57 p.m.

Description

According to the kconfig-xt example [1], it seems to me we need to call this method for settings changing. Therefore, it is not enough to just set the certain properties separately. I currently need to wrap this writeConfig() method in a helper class in order to get an invokable or slot version. There are at least two ways of addressing the issue in question: 1) Make the current method Q_INVOKABLE 2) Make the method actually a slot I personally chose the second alternative since the method already returns void, thus a potential candidate for being a slot. According to the Qt documentation, it is not a problem for having a virtual (public) slot: "You can also define slots to be virtual, which we have found quite useful in practice." [1] http://techbase.kde.org/Development/Tutorials/Using_KConfig_XT#Example

Testing

Tested on Archlinux for building only.

Diffs

  • kdecore/config/kcoreconfigskeleton.h (b42ff22)

View Diff
Permalink | Reply | 0 comments |
headers
Alexander Neundorf | 1 Feb 17:53
Picon
Favicon

Re: Review Request: Port shutdown dialog to QML

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

ksmserver/FindKDeclarative.cmake (Diff revision 9) 29
INCLUDE(${kde_cmake_module_dir}/FindPackageHandleStandardArgs.cmake)
You can simply include(FindPackageHandleStandardArgs). The full path should not be necessary. Especially not since ${kde_cmake_module_dir} is not set in this file, and I don't know where it comes from. The prefix "kde_" makes it look like it comes from FindKDE4Internal.cmake, but there everything has the prefix "KDE4_". Everything else is internal and should not be used.

ksmserver/FindKDeclarative.cmake (Diff revision 9) 33
SET(KDECLARATIVE_LIBRARIES ${KDECLARATIVE_LIBRARY})
You should also add a set(KDECLARATIVE_INCLUDE_DIRS ${KDECLARATIVE_INCLUDE_DIR}), as recommended in the cmake module readme.txt, see http://www.cmake.org/cgi-bin/viewcvs.cgi/Modules/readme.txt?root=CMake&view=markup

ksmserver/FindKDeclarative.cmake (Diff revision 9) 36
MARK_AS_ADVANCED(KDECLARATIVE_LIBRARY KDECLARATIVE_LIBRARIES KDECLARATIVE_INCLUDE_DIR )
KDECLARATIVE_LIBRARIES doesn't have to be marked as advanced, since it is not a cache variable. (the other two variables are the result variables from find_path() and find_library(), so they are put automatically in the cache, and so should be marked as advanced, as done here)

- Alexander


On January 30th, 2012, 5:08 p.m., Lamarque Vieira Souza wrote:

Review request for KDE Base Apps and KDE Runtime.
By Lamarque Vieira Souza.

Updated Jan. 30, 2012, 5:08 p.m.

Description

Port shutdown dialog to QML. Two QML themes are included: default, which mimics the current shutdown dialog look & feel, and contour, which is used in Plasma Active.

Testing

Works in Plasma Active Two using MeeGo image and KDE SC 4.8. It does not work in 4.7.x because the default theme requires kde-runtime 4.8's declarative imports. TODO: . test right to left language support.
Bugs: 216853, 216853

Diffs

  • ksmserver/CMakeLists.txt (295b96e)
  • ksmserver/Copyright.txt (PRE-CREATION)
  • ksmserver/FindKDeclarative.cmake (PRE-CREATION)
  • ksmserver/Messages.sh (0aa8bab)
  • ksmserver/shutdown.cpp (7fd1e11)
  • ksmserver/shutdowndlg.h (e5f0942)
  • ksmserver/shutdowndlg.cpp (a09a1a7)
  • ksmserver/themes/contour/ContourButton.qml (PRE-CREATION)
  • ksmserver/themes/contour/main.qml (PRE-CREATION)
  • ksmserver/themes/contour/metadata.desktop (PRE-CREATION)
  • ksmserver/themes/contour/screenshot.png (PRE-CREATION)
  • ksmserver/themes/default/ContextMenu.qml (PRE-CREATION)
  • ksmserver/themes/default/KSMButton.qml (PRE-CREATION)
  • ksmserver/themes/default/MenuItem.qml (PRE-CREATION)
  • ksmserver/themes/default/helper.js (PRE-CREATION)
  • ksmserver/themes/default/main.qml (PRE-CREATION)
  • ksmserver/themes/default/metadata.desktop (PRE-CREATION)
  • ksmserver/themes/default/screenshot.png (PRE-CREATION)

View Diff

Screenshots

Permalink | Reply | 0 comments |
headers
Dawit A | 2 Feb 01:25
Picon
Favicon

Re: Review Request: Fix a VLC crash by delaying object deletion to avoid invalid access by QtDBus

On Mon, Jan 30, 2012 at 4:48 AM, Thiago Macieira <thiago <at> kde.org> wrote:
> On Monday, 30 de January de 2012 08.05.50, Cristian Oneț wrote:
>> > Diffs
>> > -----
>> >
>> >   kio/bookmarks/kbookmarkmanager.cc d8a9cb7
>> >   kio/kio/fileundomanager.cpp f580c29
>> >   kio/kio/scheduler.cpp 6b36d9d
>> >
>> > Diff: http://git.reviewboard.kde.org/r/100577/diff/diff
>
> Do you have a valgrind trace of the problem?
>
> I cannot confirm that the fix is correct or not because I don't know why the
> crash happens.

I can generate one, but I do not have a debug build of Qt and it takes
way too long to build that on my 6 year old home PC.
Permalink | Reply | 1 comment |
headers
Anders Lund | 2 Feb 17:44
Picon
Favicon

nepomuk restarting

Hi,

I don't know if this is the right place for this, but:

I experience once in a while a need to kill off nepomuk/virtuoso-t and restart 
it. This again means that most apps depending on it must be restarted, ie 
bangarang, dolphin, gwenview and plasma-desktop is in a somewhat broken state 
after restarting nepomuk.

Would it be possible to make this happen automatically, like kdepim apps 
automatically reconnects when akonadi is restarted? Or signal that the service 
have been restarted to let applications know so that they can act?

Nepomuk itself also have some issues with handling problems. If virtuoso-t is 
killed, it is not possible to display the nepomuk  kcm.

I know nepomuk is meant to Just Work in the background, but it is a painfull 
fact that this is not always the case. Even when nepomuk becomes perfectly 
stable, it would not hurt being able to handle it getting killed.

--

-- 
Anders
Permalink | Reply | 1 comment |
headers
Dawit Alemayehu | 2 Feb 19:20
Picon
Favicon

Review Request: Prompt for the folder name when creating one in the sidebar bookmark module

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

Review request for KDE Base Apps and David Faure.
By Dawit Alemayehu.

Description

The attached patch implemets the TODO about prompting the user for a name when creating a new bookmark folder in Konqueror's sidebar bookmark widget.

Testing

Follow the steps outlined in the bug report before and after the patch.
Bugs: 247049

Diffs

  • konqueror/sidebar/trees/bookmark_module/bookmark_module.cpp (23b8e05)

View Diff
Permalink | Reply | 1 comment |
headers
Frank Reininghaus | 2 Feb 19:54

Re: Review Request: Fix a VLC crash by delaying object deletion to avoid invalid access by QtDBus

Hi,

Am 30. Januar 2012 10:48 schrieb Thiago Macieira:
> On Monday, 30 de January de 2012 08.05.50, Cristian Oneț wrote:
>> > Diffs
>> > -----
>> >
>> >   kio/bookmarks/kbookmarkmanager.cc d8a9cb7
>> >   kio/kio/fileundomanager.cpp f580c29
>> >   kio/kio/scheduler.cpp 6b36d9d
>> >
>> > Diff: http://git.reviewboard.kde.org/r/100577/diff/diff
>
> Do you have a valgrind trace of the problem?
>
> I cannot confirm that the fix is correct or not because I don't know why the
> crash happens.

I've attached a Valgind trace to the bug report:

https://bugs.kde.org/show_bug.cgi?id=234484#c25

Best regards,
Frank
Permalink | Reply | 0 comments |
headers
David Faure | 2 Feb 21:07
Picon
Favicon
Gravatar

Re: Review Request: Fix a VLC crash by delaying object deletion to avoid invalid access by QtDBus

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

A very general rule to software development is: don't commit code or fixes that you don't understand. So I don't like this, because it's a workaround for an un-identified issue. If it crashes, where's the valgrind log that explains exactly why it crashes? If it's a Qt bug, then it should be investigated and fixed in Qt. Then we can talk about short-term workarounds to make kde users happier, but not before (which would just hide and postpone the issue until later). What's the harm in committing this anyway, you might ask? Well 1) this could happen again anywhere else since this looks like a fix at the wrong level, 2) I don't know if deleteLater are processed at app quitting time, so this might lead to false positives in future memory-leak debugging. 3) in general, any of these non-running destructors could be doing things that we actually need to happen.

- David


On January 30th, 2012, 6:35 a.m., Dawit Alemayehu wrote:

Review request for kdelibs and Thiago Macieira.
By Dawit Alemayehu.

Updated Jan. 30, 2012, 6:35 a.m.

Description

As described in the bug report when opening the file dialog, canceling it and exiting VLC causes the application to seg fault. The backtraces posted in the bug report shows that the crash happens somewhere in QtDBus and I get a similar backtrace when using v1.1.7 of this application. When I looked into this issue what I discovered was that only KIO classes that register themselves with QtDBus cause the crash. I only found that out because fixing the crash in KIO::Scheduler by defering its deletion using deleteLater() did not prevent the application from still crashing on exit. However, the crash had moved to another location, KBookmarkManager. The only common thing between the crash at KBookmarkmanager and KIO::Scheduler was that in both cases the backtrace shows QtDBus attempting to invoke or access some method or slot in the object that was just destroyed. At this point I decided to check how many other classes in KIO register themselves with QtDBus and found only one more. I then applied the same changes to all three, defering the deletion of any object registered with QtDBus using QObject::deleteLater, and the crashing went away. Anyhow, I have no idea why the crashes are only seen with this application. I was not able to duplicate the crash using the same sequences in another Qt only app, Arora or any of the other KDE applications I tried.
Bugs: 234484

Diffs

  • kio/bookmarks/kbookmarkmanager.cc (d8a9cb7)
  • kio/kio/fileundomanager.cpp (f580c29)
  • kio/kio/scheduler.cpp (6b36d9d)

View Diff
Permalink | Reply | 0 comments |
headers
Xuetian Weng | 2 Feb 16:23
Picon
Gravatar

Review Request: Add recentdocuments:/ kio slave to kde-runtime.

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

Review request for KDE Runtime.
By Xuetian Weng.

Description

Add recentdocuments:/ kio slave to kde-runtime. Did some little rename "recentdocument -> recentdocuments", based on http://kde-apps.org/content/show.php?content=145878

Testing

Works for me.

Diffs

  • kioslave/CMakeLists.txt (f3d5b00)
  • kioslave/recentdocuments/CMakeLists.txt (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.h (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.cpp (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.protocol (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.h (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.cpp (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.desktop (PRE-CREATION)

View Diff
Permalink | Reply | 11 comments |
headers
Albert Astals Cid | 3 Feb 00:27
Picon
Favicon

Re: Review Request: Add recentdocuments:/ kio slave to kde-runtime.

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

You need a Messages.sh to extract your i18n calls so that they can be translated
kioslave/recentdocuments/recentdocuments.cpp (Diff revision 1) 22
extern "C" int KDE_EXPORT kdemain(int argc, char **argv)
KComponentData("kio_recentdocuments", "kdelibs4");
That kdelibs4 does not make sense, you need your own translation catalog, loading the kdelibs4 catalog only won't help

kioslave/recentdocuments/recentdocuments.protocol (Diff revision 1) 13
maxInstances=4
Why 4 maxInstances?

- Albert


On February 2nd, 2012, 3:23 p.m., Xuetian Weng wrote:

Review request for KDE Runtime.
By Xuetian Weng.

Updated Feb. 2, 2012, 3:23 p.m.

Description

Add recentdocuments:/ kio slave to kde-runtime. Did some little rename "recentdocument -> recentdocuments", based on http://kde-apps.org/content/show.php?content=145878

Testing

Works for me.

Diffs

  • kioslave/CMakeLists.txt (f3d5b00)
  • kioslave/recentdocuments/CMakeLists.txt (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.h (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.cpp (PRE-CREATION)
  • kioslave/recentdocuments/recentdocuments.protocol (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.h (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.cpp (PRE-CREATION)
  • kioslave/recentdocuments/recentdocumentsnotifier.desktop (PRE-CREATION)

View Diff
Permalink | Reply | 1 comment |
headers
Stefan Majewsky | 3 Feb 14:51

Re: nepomuk restarting

bugs.kde.org?

Greetings
Stefan
Permalink | Reply | 0 comments |

Gmane