Re: detection if applet is running

2011-10-28 Thread Aaron J. Seigo
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)

2011-10-28 Thread Sebastian Trüg
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

2011-10-28 Thread Christoph Feck


 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)

2011-10-28 Thread Albert Astals Cid
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

2011-10-28 Thread Andriy Rysin

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)

2011-10-28 Thread Kevin Kofler
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

2011-10-28 Thread Andriy Rysin

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)

2011-10-28 Thread Christoph Feck
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

2011-10-28 Thread Aaron J. Seigo
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

2011-10-28 Thread Greg T


 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)

2011-10-28 Thread Thiago Macieira
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)

2011-10-28 Thread David Jarvie
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)

2011-10-28 Thread Rex Dieter
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