Re: detection if applet is running
On Thursday, October 27, 2011 23:35:52 Andriy Rysin wrote: I did a quick research and I see two ways to solve this in keyboard_layout_widget (embedded component used in lockdlg): 1) link plasma libs and do something like (in pseudo-code) foreach(containment, new Corona()-containments()) { foreach(applet, containment-applets()) { if( applet-pluginName() == ... ) { // show indicator } } } that won't work out of the plasma-desktop process, so this isn't an option for you. 2) make keyboard layout applet exposes some dbus to make it detectable when running (feels a bit too heavy) that would be a preferred way ... note, however, that one can have multiple instances of the same applet. this is why we really recommend and encourage a separation between visualization and business logic, where the various visualizations (e.g. applets, tray icons, etc) all share the same business logic code (e.g. a DataEngine) which can then take care of things like registering a unique dbus interface. It felt like there must be a way currently to use dbus on plasma to query running applets (which would the easiest way to implement what I need) nobody has written such a thing, nor do i think it would be as easy as one may think to use as a widget may be running not in plasma-desktop but in plasma- netbook, plasma-active, plasma-windowed, etc. and it may be in any number of running containments. it is much easier and safer to simply expose a dbus service from your DataEngine, or whatever the DataEngine uses to get keyboard information from, than any other approach. it guarantees it works with multiple widgets and doesn't care which shell the user is running. in this particular case, why don't you just check the keyboard layout settings to see if more than one keyboard layout is active? does it matter that there is or isn't a keyboard layout switcher applet or icon? note that in plasma active, it is very likely that this functionality will end up integrated into the virtual keyboard and not reside as a separate applet in the containments running in plasma-active ... -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On 10/27/2011 11:35 PM, Thiago Macieira wrote: On Thursday, 27 de October de 2011 23:17:49 Milian Wolff wrote: On Thursday 27 October 2011 21:11:11 Thiago Macieira wrote: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. is there at least a qWarning emitted in such a case, so we can find these problems with QT_FATAL_WARNINGS=1 ? No, but there's something better: #define QURL_NO_CAST_FROM_QSTRING Your code will not compile when you're auto-casting. You'll need to select what to do: QUrl::fromEncoded() - if it's a URL, with the file scheme QUrl::fromLocalFile() - if it's a file name This option remains in Qt 5, but there there's going to be a new method to convert from QString to QUrl without passing through QByteArray. This is extremely important to get right because a filename and a URL are NOT the same. Certain characters in the string are interpreted differently depending on what the string is: %, # and ? in particular. So, to be honest, the bug *already* *existed* in your code if you're finding these problems now. I just made it blatantly clear, and it's been there for a year for people to see. I'm also calling right now KUrl's fromPathOrUrl a fatally flawed design. Be that as it may but the fact remains that KDE potentially does not work with Qt 4.8. Is that really a risk worth taking? I am all for fixed semantics in the methods but this seems like a problematic case. Cheers Sebastian
Re: Review Request: kfileplaceeditdialog lineedit too small
On Oct. 5, 2011, 11:30 a.m., David Faure wrote: Why the setMaxLength?? What if one wants to type in a long URL? Also, I can't reproduce the bug here (kde-4.7), but maybe only because the big icon button makes the dialog quite large? Greg T wrote: indeed, the setmaxLength was stupid it's quite small on 4.6.5 http://wstaw.org/m/2011/10/05/plasma-desktopF27745.jpg David Faure wrote: Ah I see, it's the german translation which makes the label longer (Choose an icon - Wählen Sie ein Symbol aus), leaving less room for the lineedits. Patch 3 looks good, although I can't help but think there's a bug if a set*Width() call takes a height() as value :-) But I'll trust Christoph on this one. Peter Penz wrote: Patch 3 looks good, although I can't help but think there's a bug if a set*Width() call takes a height() as value :-) But I'll trust Christoph on this one. Using QFontMetrics::averageCharWidth() might be an alternative in comparison to use the height as reference for calculating the width (but probably this has some drawbacks I'm not aware of). Christoph Feck wrote: First, averageCharWidth() is slow. It goes through every character to compute the average. Second, that average might be skewed when there are broken characters in the font. I'll add a comment why the lineedits width depends on the fonts height. Greg T wrote: Hey is your patch already submitted? Yes, see the bug report. But appearantly, the commit hook did not close the review request. Please mark it as submitted. - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102751/#review7117 --- On Oct. 5, 2011, 6:22 p.m., Greg T wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102751/ --- (Updated Oct. 5, 2011, 6:22 p.m.) Review request for Dolphin and kdelibs. Description --- Hi, this patch sets a minimum size for the widget. This addresses bug 266435. http://bugs.kde.org/show_bug.cgi?id=266435 Diffs - kfile/kfileplaceeditdialog.cpp d798b4d Diff: http://git.reviewboard.kde.org/r/102751/diff/diff Testing --- it's working Thanks, Greg T
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
A Divendres, 28 d'octubre de 2011, Thiago Macieira vàreu escriure: On Friday, 28 de October de 2011 10:41:36 Sebastian Trüg wrote: So, to be honest, the bug already existed in your code if you're finding these problems now. I just made it blatantly clear, and it's been there for a year for people to see. I'm also calling right now KUrl's fromPathOrUrl a fatally flawed design. Be that as it may but the fact remains that KDE potentially does not work with Qt 4.8. Is that really a risk worth taking? I am all for fixed semantics in the methods but this seems like a problematic case. Let me repeat: the problem already existed. This just made the broken code more evident. You should fix it ANYWAY. Right, it was wrong if it had to accept user input but for internal hardcoded paths, it worked and now it does not. Albert If you had a file path of: /tmp/Who let the dogs out?.mp3 or/tmp/Mambo #5.mp3 In Qt 4.7, toLocalFile on those URLs above would return: /tmp/Who let the dogs out and /tmp/Mambo Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
Re: detection if applet is running
On 10/28/2011 09:20 AM, Ruurd Pels wrote: On Thursday 27 October 2011 23:35:52 Andriy Rysin wrote: 2) make keyboard layout applet exposes some dbus to make it detectable when running (feels a bit too heavy) Yes. Better. Why does it feel heavy? I'd venture the thought that doing so could expose the keybaord layout applet in such a way that it also could do something usefull (like changing layouts for example :-) ) We already have that dbus API in keyboard kded daemon Adding dbus API to applet just to tell if it is running seems wrong to me it feels like this belongs to parent or container Imagine if to check that app is running in Linux you have to add some code to each app :)
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. +1 You just CANNOT change the behavior of an existing function in such a way. Kevin Kofler
Re: detection if applet is running
On 10/28/2011 02:38 AM, Aaron J. Seigo wrote: On Thursday, October 27, 2011 23:35:52 Andriy Rysin wrote: 2) make keyboard layout applet exposes some dbus to make it detectable when running (feels a bit too heavy) that would be a preferred way ... note, however, that one can have multiple instances of the same applet. this is why we really recommend and encourage a separation between visualization and business logic, where the various visualizations (e.g. applets, tray icons, etc) all share the same business logic code (e.g. a DataEngine) which can then take care of things like registering a unique dbus interface. there's no business logic in the applet, its data engine is x.org it feels to me that adding a data engine just to be able to tell if applet is running is an overkill it is much easier and safer to simply expose a dbus service from your DataEngine, or whatever the DataEngine uses to get keyboard information from, than any other approach. it guarantees it works with multiple widgets and doesn't care which shell the user is running. in this particular case, why don't you just check the keyboard layout settings to see if more than one keyboard layout is active? does it matter that there is or isn't a keyboard layout switcher applet or icon? note that in plasma active, it is very likely that this functionality will end up integrated into the virtual keyboard and not reside as a separate applet in the containments running in plasma-active ... At first I thought it might be a bit inconsistent if user didn't set and indicator to see in the desktop but will see it in the lock dialog, but that's probably an edge case and most of the time user wants some indication of current layout in the lock dialog. That's what I'll do - always show indicator if layout count 1. Thanks for the suggestion, Andriy
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Friday 28 October 2011 17:30:44 Kevin Kofler wrote: Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. +1 You just CANNOT change the behavior of an existing function in such a way. Kevin Kofler Give me a break. I am running KDE/master with Qt 4.8 branch since some time, and if this change would cause bugs to show up everwhere the toLocalFile() method is used, then I certainly had noticed this change. From the discussion so far it looks like only KNotify is affected, and I am sure KNotify can be fixed to adapt to the Qt 4.8 change. If other issues surface, they will be noticed, because an empty file name certainly cannot work at all. Bugs because of missing characters in file names (as Thiago pointed out) are harder to see. Additionally, this certainly is not the first time an update to Qt caused regression we had to fix. Starting from source-incompatible changes and ending with issues with the raster graphics system - we went through all of them; and for the better. If we insist bug-for-bug compatibility in Qt for the next years, I am sure Qt developers will happily stop fixing any bugs. Christoph Feck (kdepepo) KDE Quality Team
Re: detection if applet is running
On Friday, October 28, 2011 11:26:05 Andriy Rysin wrote: We already have that dbus API in keyboard kded daemon could the kded daemon be checked to see if keyboard layouts are available, and if so, show that button in the unlock dialog? does it really need to rely on whether or not there is a system tray icon or plasmoid? -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part.
Re: Review Request: kfileplaceeditdialog lineedit too small
On Oct. 5, 2011, 11:30 a.m., David Faure wrote: Why the setMaxLength?? What if one wants to type in a long URL? Also, I can't reproduce the bug here (kde-4.7), but maybe only because the big icon button makes the dialog quite large? Greg T wrote: indeed, the setmaxLength was stupid it's quite small on 4.6.5 http://wstaw.org/m/2011/10/05/plasma-desktopF27745.jpg David Faure wrote: Ah I see, it's the german translation which makes the label longer (Choose an icon - Wählen Sie ein Symbol aus), leaving less room for the lineedits. Patch 3 looks good, although I can't help but think there's a bug if a set*Width() call takes a height() as value :-) But I'll trust Christoph on this one. Peter Penz wrote: Patch 3 looks good, although I can't help but think there's a bug if a set*Width() call takes a height() as value :-) But I'll trust Christoph on this one. Using QFontMetrics::averageCharWidth() might be an alternative in comparison to use the height as reference for calculating the width (but probably this has some drawbacks I'm not aware of). Christoph Feck wrote: First, averageCharWidth() is slow. It goes through every character to compute the average. Second, that average might be skewed when there are broken characters in the font. I'll add a comment why the lineedits width depends on the fonts height. Hey is your patch already submitted? - Greg --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102751/#review7117 --- On Oct. 5, 2011, 6:22 p.m., Greg T wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102751/ --- (Updated Oct. 5, 2011, 6:22 p.m.) Review request for Dolphin and kdelibs. Description --- Hi, this patch sets a minimum size for the widget. This addresses bug 266435. http://bugs.kde.org/show_bug.cgi?id=266435 Diffs - kfile/kfileplaceeditdialog.cpp d798b4d Diff: http://git.reviewboard.kde.org/r/102751/diff/diff Testing --- it's working Thanks, Greg T
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Friday, 28 de October de 2011 17:43:32 Kevin Kofler wrote: Thiago Macieira wrote: Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. Such a change might make sense for 5.0, but not for 4.8! Or if you really want to add the changed API to 4.x, you have to do it with a new name, deprecating (but not removing!) toLocalFile. Let's just say I disagree. This change was for 4.7, but it was reverted and added to 4.8, giving you well over ONE YEAR to adapt. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Saturday 29 October 2011 00:05:18 Thiago Macieira wrote: On Friday, 28 de October de 2011 17:43:32 Kevin Kofler wrote: Thiago Macieira wrote: Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. Such a change might make sense for 5.0, but not for 4.8! Or if you really want to add the changed API to 4.x, you have to do it with a new name, deprecating (but not removing!) toLocalFile. Let's just say I disagree. This change was for 4.7, but it was reverted and added to 4.8, giving you well over ONE YEAR to adapt. Only for those who knew about it a year ago. Many people will probably only find out when things don't work with 4.8. -- David Jarvie. KDE developer. KAlarm author -- http://www.astrojar.org.uk/kalarm
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Milian Wolff wrote: Could someone maybe explain a few points on this issue for me? 1) When does it manifest? Apparently when using QUrl(...) directly, if I'm not mistaken. But what if we use KUrl? KUrl uses QUrl behind the scenes for several cases. -- rex