Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread Martin Gräßlin

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

Ship it!


Looks good to me, though I can of course not comment on the platform specific 
code.

- Martin Gräßlin


On Sept. 25, 2014, 6:14 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 6:14 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread Thiago Macieira
On Thursday 25 September 2014 22:32:35 René J.V. Bertin wrote:
> Ah, apparently Thiago missed the multiple recent reminders of the fact that
> we OS X users are going to be "stuck" in/with KDE4 for a tad longer than
> the average Linux user (Debian apart, maybe? ;)) Anyway, I guess I'm going
> to have to look again at how exactly `GUIenabled=false` leads to the
> qt_menu.nib not being loaded in Qt ...

Note that you're applying the change to X11 too and that will have an impact, 
but limited lifetime. If the problem is Mac only, then maybe apply the 
solution to Mac only too.

And still it needs to be studied for Qt5, unless the plan is to never 
transition to Qt 5 (which means KDE-on-Mac will stop compiling within 2 
years).
-- 
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: Review Request 120376: drKonqi Fix Bug 337742 - Unable to send report: error code 410 from Bugzilla

2014-09-25 Thread Frédéric Sheedy

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

(Updated sep. 26, 2014, 4:21 matin)


Review request for KDE Runtime.


Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742


Repository: kde-runtime


Description
---

Bugzilla 4.4.5 now request token to create report.


Diffs
-

  drkonqi/bugzillalib.h 570169b 
  drkonqi/bugzillalib.cpp f74753c 
  drkonqi/tests/bugzillalibtest/bugzillalibtest.cpp 46c95b6 

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


Testing
---

Testing was done using crashtest program in drkonqui folder on 
https://bugstest.kde.org
Create new bug report and add attachement works now.


Thanks,

Frédéric Sheedy



Review Request 120376: drKonqi Fix Bug 337742 - Unable to send report: error code 410 from Bugzilla

2014-09-25 Thread Frédéric Sheedy

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

Review request for KDE Runtime.


Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742


Repository: kde-runtime


Description
---

Bugzilla 4.4.5 now request token to create report.


Diffs
-

  drkonqi/bugzillalib.h 570169b 
  drkonqi/bugzillalib.cpp f74753c 
  drkonqi/tests/bugzillalibtest/bugzillalibtest.cpp 46c95b6 

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


Testing
---

Testing was done using crashtest program in drkonqui folder on 
https://bugstest.kde.org
Create new bug report and add attachement works now.


Thanks,

Frédéric Sheedy



Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread René J . V . Bertin


> On Sept. 25, 2014, 8:07 p.m., Thomas Lübking wrote:
> > Aside Thiagos concern, what actually causes the menubar/docker item stuff?
> > 
> > Quoting QApplication (4.8):
> > > On X11, the window system is initialized if GUIenabled is true. If 
> > > GUIenabled is false, the application does not connect to the X server. On 
> > > Windows and Mac OS, currently the window system is always initialized, 
> > > regardless of the value of GUIenabled. This may change in future versions 
> > > of Qt.
> > 
> > So it's perhaps rather a statement in KApplicationPrivate::init() which 
> > causes this?
> > There aren't too many depending on GUIEnabled ... ;-)
> 
> René J.V. Bertin wrote:
> Thiagos?
> 
> The menubar/Dock "item stuff" is the result of `qt_mac_loadMenuNib` in 
> `qapplication_mac.mm`, which is called when 
> `!QApplication::testAttribute(Qt::AA_MacPluginApplication)`. Setting 
> `GUIenabled=false` has that effect, ultimately.
> 
> Thomas Lübking wrote:
> > Thiagos?
> 
> By mail:
> http://lists.kde.org/?l=kde-core-devel&m=141166670217317&w=2
> 
> > result of qt_mac_loadMenuNib in qapplication_mac.mm
> 
> Ok, so the API doc lied to me ;-)
> Since the attributes are static, you could simply set it before creating 
> the application in the relevant main funcs, could you?
> (Though i actually don't see it set anywhere - but it's late and i'm 
> tired...)

Ah, apparently Thiago missed the multiple recent reminders of the fact that we 
OS X users are going to be "stuck" in/with KDE4 for a tad longer than the 
average Linux user (Debian apart, maybe? ;))
Anyway, I guess I'm going to have to look again at how exactly 
`GUIenabled=false` leads to the qt_menu.nib not being loaded in Qt ...

I don't think the API docs lied to you. Not loading the menu nib doesn't mean 
that the window system is not initialised ...


- René J.V.


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


On Sept. 25, 2014, 3:32 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120363/
> ---
> 
> (Updated Sept. 25, 2014, 3:32 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Applications can be defined in their CMake file as being `NOGUI`, but until 
> now this has had very limited effect. Especially on OS X, those applications 
> can still construct a minimal GUI and thus have "visual presence" in the Dock 
> and application switcher (and have a menubar as well).
> 
> This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for 
> those targets, and uses that token to set the default value for the 
> `GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.
> 
> This could potentially be combined on OS X with the CoreFoundation call that 
> turns a running application into an "agent" (see 
> https://git.reviewboard.kde.org/r/120354).
> 
> 
> Diffs
> -
> 
>   cmake/modules/KDE4Macros.cmake 073d726 
>   kdeui/kernel/kapplication.h fa2ab26 
>   kdeui/kernel/kapplication.cpp b093034 
>   kdeui/kernel/kuniqueapplication.h e05dcd7 
> 
> Diff: https://git.reviewboard.kde.org/r/120363/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14), rebuilt kdelibs, 
> kde-workspace, kde-runtime, kde-baseapps, kdepim-runtime and nepomuk-core.
> If the documentation I read is correct, the `GUIenabled` switch has no effect 
> on Linux, so this patch shouldn't have any either on that OS.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

2014-09-25 Thread René J . V . Bertin


> On Sept. 24, 2014, 7:48 p.m., Thomas Lübking wrote:
> > I assume you'd be better off altering KMenu::addTitle() - or even patch Qt 
> > (QMenu on mach cannot deal w/ widget actions, at least if used on the 
> > global menubar)
> 
> René J.V. Bertin wrote:
> I agree totally, but for that
> 
> - I'd have to understand exactly what the addTitle does that makes Qt/Mac 
> crash
> - Ideally I'd also know how to determine if the menu is in the global 
> menubar or e.g. in a popup menu, where addTitle works perfectly fine. I think 
> we'd want to preserve that because popup menus follow the selected style and 
> not necessarily the OS X style.
> 
> There's also the point that the addTitle (and addSection, IIRC) in Qt5 
> don't crash. They have other issues (IIRC you get just a separator, not the 
> title text) but until now I've preferred to handle these crashes on a 
> case-by-case basis.
> 
> I admit, this RR was also made a bit with the idea of getting a 
> discussion going about this issue. ;)
> 
> Thomas Lübking wrote:
> Since KMenu is deprecated and the ::addTitle() implementation doesn't 
> differ in KF5, either the applications have simply been ported away from 
> KMenu or QWidgetAction was fixed in Qt5.
> 
> To know why exactly this crashes for you, i'd need to see a backtrace 
> (paste.kde.org) - Qt4 claimed QWidgetAction support on OSX' global menu - 
> with some caveats.
> If QMenu::menuAction() is in the action list of the global menu - 
> unfortunately, this menubar is parentless :-(
> Also there's no guarantee that this assignment won't change at some point 
> in the future to any direction.
> 
> > IIRC you get just a separator, not the title text
> 
> What basically means that the QWidget(Action) reparenting doesn't work at 
> all in Qt5 anymore (at best the linked out widget is just hidden)
> 
> 
> Disclaimer: I'm a bit biased here ;-)
> Imo using a QWidgetAction as title was a wrong design itfp - I proposed a 
> Qt4 patch to use a leading and entitled separator instead, but it was 
> rejected because not all styles did/do support texted separators. No idea 
> whether that patch was revived for Qt5, never tested. (And, tbh, I don't know 
> whether the native styles, ie. Win and Mac, support texted separators)
> 
> René J.V. Bertin wrote:
> backtrace: http://paste.kde.org/pvnu8pgui
> 
> If I recall correctly, Qt5.3's QMenu::addTitle and QMenu::addSection 
> indeed call for what I think you mean with texted separators. And OS X will 
> only render the separator for those. OS X 10.6 in any case, but I don't see 
> why that would have changed in later versions.
> 
> Thomas Lübking wrote:
> Thanks.
> QMenu::addTitle() does not exist in 5.3 and ::setTitle() refers to the 
> menubar item text.
> ::addSection() might work (if the building loop was reversed, making a 
> separator as first element possible ;-)
> 
> On the crash:
> It occurs because QWidgetPrivate::setGeometry_sys_helper() in 
> qwidget_mac.mm is not aware that the widget it operates on is a toplevel 
> widget (and has no parent)
> This seems to be the "QMacNativeWidget(0);" "container" created in 
> qmenu_mac.mm, QMenuPrivate::QMacMenuPrivate::addAction()
> 
> Why it doesn't figure so, I don't know, but assume that in
> 
> ```cpp
> bool QWidgetPrivate::isRealWindow() const
> {
> return q_func()->isWindow() && !topData()->embedded;
> }
> ```
> 
> "topData()->embedded" will be true (so the return be false)
> 
> René J.V. Bertin wrote:
> Hmm, it's been a while that I looked at that - when making kmail "not 
> crash" because of the same reason on OS X. I never submitted a patch for that 
> here because I noticed that kdepim git/master used new QMenu functions. I 
> ported over QMenu::addSection to KMenu, and that's where I saw that a "texted 
> separator" remains just a separator on OS X.
> 
> Are you sure a menubar becomes the global menubar only when it doesn't 
> have a parent? I seem to recall that the situation is a little bit more 
> complex than that.
> 
> Thomas Lübking wrote:
> > Are you sure a menubar becomes the global menubar only when it doesn't 
> have a parent? I seem to recall that the situation is a little bit more 
> complex than that.
> 
> I can only quote the Qt docs on this:
> 
> > If you want all windows in a Mac application to share one menu bar, you 
> must create a menu bar that does not have a parent. Create a parent-less menu 
> bar this way:
> > QMenuBar *menuBar = new QMenuBar(0);
> > Note: Do not call QMainWindow::menuBar() to create the shared menu bar, 
> because that menu bar will have the QMainWindow as its parent. That menu bar 
> would only be displayed for the parent QMainWindow.
> 
> This has however nothing to do with the crash - it's the 
> "QMacNativeWidget" which has no parent but is

Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

2014-09-25 Thread Thomas Lübking


> On Sept. 24, 2014, 5:48 nachm., Thomas Lübking wrote:
> > I assume you'd be better off altering KMenu::addTitle() - or even patch Qt 
> > (QMenu on mach cannot deal w/ widget actions, at least if used on the 
> > global menubar)
> 
> René J.V. Bertin wrote:
> I agree totally, but for that
> 
> - I'd have to understand exactly what the addTitle does that makes Qt/Mac 
> crash
> - Ideally I'd also know how to determine if the menu is in the global 
> menubar or e.g. in a popup menu, where addTitle works perfectly fine. I think 
> we'd want to preserve that because popup menus follow the selected style and 
> not necessarily the OS X style.
> 
> There's also the point that the addTitle (and addSection, IIRC) in Qt5 
> don't crash. They have other issues (IIRC you get just a separator, not the 
> title text) but until now I've preferred to handle these crashes on a 
> case-by-case basis.
> 
> I admit, this RR was also made a bit with the idea of getting a 
> discussion going about this issue. ;)
> 
> Thomas Lübking wrote:
> Since KMenu is deprecated and the ::addTitle() implementation doesn't 
> differ in KF5, either the applications have simply been ported away from 
> KMenu or QWidgetAction was fixed in Qt5.
> 
> To know why exactly this crashes for you, i'd need to see a backtrace 
> (paste.kde.org) - Qt4 claimed QWidgetAction support on OSX' global menu - 
> with some caveats.
> If QMenu::menuAction() is in the action list of the global menu - 
> unfortunately, this menubar is parentless :-(
> Also there's no guarantee that this assignment won't change at some point 
> in the future to any direction.
> 
> > IIRC you get just a separator, not the title text
> 
> What basically means that the QWidget(Action) reparenting doesn't work at 
> all in Qt5 anymore (at best the linked out widget is just hidden)
> 
> 
> Disclaimer: I'm a bit biased here ;-)
> Imo using a QWidgetAction as title was a wrong design itfp - I proposed a 
> Qt4 patch to use a leading and entitled separator instead, but it was 
> rejected because not all styles did/do support texted separators. No idea 
> whether that patch was revived for Qt5, never tested. (And, tbh, I don't know 
> whether the native styles, ie. Win and Mac, support texted separators)
> 
> René J.V. Bertin wrote:
> backtrace: http://paste.kde.org/pvnu8pgui
> 
> If I recall correctly, Qt5.3's QMenu::addTitle and QMenu::addSection 
> indeed call for what I think you mean with texted separators. And OS X will 
> only render the separator for those. OS X 10.6 in any case, but I don't see 
> why that would have changed in later versions.
> 
> Thomas Lübking wrote:
> Thanks.
> QMenu::addTitle() does not exist in 5.3 and ::setTitle() refers to the 
> menubar item text.
> ::addSection() might work (if the building loop was reversed, making a 
> separator as first element possible ;-)
> 
> On the crash:
> It occurs because QWidgetPrivate::setGeometry_sys_helper() in 
> qwidget_mac.mm is not aware that the widget it operates on is a toplevel 
> widget (and has no parent)
> This seems to be the "QMacNativeWidget(0);" "container" created in 
> qmenu_mac.mm, QMenuPrivate::QMacMenuPrivate::addAction()
> 
> Why it doesn't figure so, I don't know, but assume that in
> 
> ```cpp
> bool QWidgetPrivate::isRealWindow() const
> {
> return q_func()->isWindow() && !topData()->embedded;
> }
> ```
> 
> "topData()->embedded" will be true (so the return be false)
> 
> René J.V. Bertin wrote:
> Hmm, it's been a while that I looked at that - when making kmail "not 
> crash" because of the same reason on OS X. I never submitted a patch for that 
> here because I noticed that kdepim git/master used new QMenu functions. I 
> ported over QMenu::addSection to KMenu, and that's where I saw that a "texted 
> separator" remains just a separator on OS X.
> 
> Are you sure a menubar becomes the global menubar only when it doesn't 
> have a parent? I seem to recall that the situation is a little bit more 
> complex than that.

> Are you sure a menubar becomes the global menubar only when it doesn't have a 
> parent? I seem to recall that the situation is a little bit more complex than 
> that.

I can only quote the Qt docs on this:

> If you want all windows in a Mac application to share one menu bar, you must 
> create a menu bar that does not have a parent. Create a parent-less menu bar 
> this way:
> QMenuBar *menuBar = new QMenuBar(0);
> Note: Do not call QMainWindow::menuBar() to create the shared menu bar, 
> because that menu bar will have the QMainWindow as its parent. That menu bar 
> would only be displayed for the parent QMainWindow.

This has however nothing to do with the crash - it's the "QMacNativeWidget" 
which has no parent but is treated by ::setGeometry_sys_helper() as if it had.
The call to ::invalidateBuf

Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread Thomas Lübking


> On Sept. 25, 2014, 6:07 nachm., Thomas Lübking wrote:
> > Aside Thiagos concern, what actually causes the menubar/docker item stuff?
> > 
> > Quoting QApplication (4.8):
> > > On X11, the window system is initialized if GUIenabled is true. If 
> > > GUIenabled is false, the application does not connect to the X server. On 
> > > Windows and Mac OS, currently the window system is always initialized, 
> > > regardless of the value of GUIenabled. This may change in future versions 
> > > of Qt.
> > 
> > So it's perhaps rather a statement in KApplicationPrivate::init() which 
> > causes this?
> > There aren't too many depending on GUIEnabled ... ;-)
> 
> René J.V. Bertin wrote:
> Thiagos?
> 
> The menubar/Dock "item stuff" is the result of `qt_mac_loadMenuNib` in 
> `qapplication_mac.mm`, which is called when 
> `!QApplication::testAttribute(Qt::AA_MacPluginApplication)`. Setting 
> `GUIenabled=false` has that effect, ultimately.

> Thiagos?

By mail:
http://lists.kde.org/?l=kde-core-devel&m=141166670217317&w=2

> result of qt_mac_loadMenuNib in qapplication_mac.mm

Ok, so the API doc lied to me ;-)
Since the attributes are static, you could simply set it before creating the 
application in the relevant main funcs, could you?
(Though i actually don't see it set anywhere - but it's late and i'm tired...)


- Thomas


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


On Sept. 25, 2014, 1:32 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120363/
> ---
> 
> (Updated Sept. 25, 2014, 1:32 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Applications can be defined in their CMake file as being `NOGUI`, but until 
> now this has had very limited effect. Especially on OS X, those applications 
> can still construct a minimal GUI and thus have "visual presence" in the Dock 
> and application switcher (and have a menubar as well).
> 
> This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for 
> those targets, and uses that token to set the default value for the 
> `GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.
> 
> This could potentially be combined on OS X with the CoreFoundation call that 
> turns a running application into an "agent" (see 
> https://git.reviewboard.kde.org/r/120354).
> 
> 
> Diffs
> -
> 
>   cmake/modules/KDE4Macros.cmake 073d726 
>   kdeui/kernel/kapplication.h fa2ab26 
>   kdeui/kernel/kapplication.cpp b093034 
>   kdeui/kernel/kuniqueapplication.h e05dcd7 
> 
> Diff: https://git.reviewboard.kde.org/r/120363/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14), rebuilt kdelibs, 
> kde-workspace, kde-runtime, kde-baseapps, kdepim-runtime and nepomuk-core.
> If the documentation I read is correct, the `GUIenabled` switch has no effect 
> on Linux, so this patch shouldn't have any either on that OS.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

2014-09-25 Thread René J . V . Bertin


> On Sept. 24, 2014, 7:48 p.m., Thomas Lübking wrote:
> > I assume you'd be better off altering KMenu::addTitle() - or even patch Qt 
> > (QMenu on mach cannot deal w/ widget actions, at least if used on the 
> > global menubar)
> 
> René J.V. Bertin wrote:
> I agree totally, but for that
> 
> - I'd have to understand exactly what the addTitle does that makes Qt/Mac 
> crash
> - Ideally I'd also know how to determine if the menu is in the global 
> menubar or e.g. in a popup menu, where addTitle works perfectly fine. I think 
> we'd want to preserve that because popup menus follow the selected style and 
> not necessarily the OS X style.
> 
> There's also the point that the addTitle (and addSection, IIRC) in Qt5 
> don't crash. They have other issues (IIRC you get just a separator, not the 
> title text) but until now I've preferred to handle these crashes on a 
> case-by-case basis.
> 
> I admit, this RR was also made a bit with the idea of getting a 
> discussion going about this issue. ;)
> 
> Thomas Lübking wrote:
> Since KMenu is deprecated and the ::addTitle() implementation doesn't 
> differ in KF5, either the applications have simply been ported away from 
> KMenu or QWidgetAction was fixed in Qt5.
> 
> To know why exactly this crashes for you, i'd need to see a backtrace 
> (paste.kde.org) - Qt4 claimed QWidgetAction support on OSX' global menu - 
> with some caveats.
> If QMenu::menuAction() is in the action list of the global menu - 
> unfortunately, this menubar is parentless :-(
> Also there's no guarantee that this assignment won't change at some point 
> in the future to any direction.
> 
> > IIRC you get just a separator, not the title text
> 
> What basically means that the QWidget(Action) reparenting doesn't work at 
> all in Qt5 anymore (at best the linked out widget is just hidden)
> 
> 
> Disclaimer: I'm a bit biased here ;-)
> Imo using a QWidgetAction as title was a wrong design itfp - I proposed a 
> Qt4 patch to use a leading and entitled separator instead, but it was 
> rejected because not all styles did/do support texted separators. No idea 
> whether that patch was revived for Qt5, never tested. (And, tbh, I don't know 
> whether the native styles, ie. Win and Mac, support texted separators)
> 
> René J.V. Bertin wrote:
> backtrace: http://paste.kde.org/pvnu8pgui
> 
> If I recall correctly, Qt5.3's QMenu::addTitle and QMenu::addSection 
> indeed call for what I think you mean with texted separators. And OS X will 
> only render the separator for those. OS X 10.6 in any case, but I don't see 
> why that would have changed in later versions.
> 
> Thomas Lübking wrote:
> Thanks.
> QMenu::addTitle() does not exist in 5.3 and ::setTitle() refers to the 
> menubar item text.
> ::addSection() might work (if the building loop was reversed, making a 
> separator as first element possible ;-)
> 
> On the crash:
> It occurs because QWidgetPrivate::setGeometry_sys_helper() in 
> qwidget_mac.mm is not aware that the widget it operates on is a toplevel 
> widget (and has no parent)
> This seems to be the "QMacNativeWidget(0);" "container" created in 
> qmenu_mac.mm, QMenuPrivate::QMacMenuPrivate::addAction()
> 
> Why it doesn't figure so, I don't know, but assume that in
> 
> ```cpp
> bool QWidgetPrivate::isRealWindow() const
> {
> return q_func()->isWindow() && !topData()->embedded;
> }
> ```
> 
> "topData()->embedded" will be true (so the return be false)

Hmm, it's been a while that I looked at that - when making kmail "not crash" 
because of the same reason on OS X. I never submitted a patch for that here 
because I noticed that kdepim git/master used new QMenu functions. I ported 
over QMenu::addSection to KMenu, and that's where I saw that a "texted 
separator" remains just a separator on OS X.

Are you sure a menubar becomes the global menubar only when it doesn't have a 
parent? I seem to recall that the situation is a little bit more complex than 
that.


- René J.V.


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


On Sept. 25, 2014, 4 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120355/
> ---
> 
> (Updated Sept. 25, 2014, 4 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt 
> KDE.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> Mac OS X cannot handle the formatting used for title menu items when it 
> applies to items in the toplevel menu bar. An application calling 

Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread René J . V . Bertin


> On Sept. 25, 2014, 8:07 p.m., Thomas Lübking wrote:
> > Aside Thiagos concern, what actually causes the menubar/docker item stuff?
> > 
> > Quoting QApplication (4.8):
> > > On X11, the window system is initialized if GUIenabled is true. If 
> > > GUIenabled is false, the application does not connect to the X server. On 
> > > Windows and Mac OS, currently the window system is always initialized, 
> > > regardless of the value of GUIenabled. This may change in future versions 
> > > of Qt.
> > 
> > So it's perhaps rather a statement in KApplicationPrivate::init() which 
> > causes this?
> > There aren't too many depending on GUIEnabled ... ;-)

Thiagos?

The menubar/Dock "item stuff" is the result of `qt_mac_loadMenuNib` in 
`qapplication_mac.mm`, which is called when 
`!QApplication::testAttribute(Qt::AA_MacPluginApplication)`. Setting 
`GUIenabled=false` has that effect, ultimately.


- René J.V.


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


On Sept. 25, 2014, 3:32 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120363/
> ---
> 
> (Updated Sept. 25, 2014, 3:32 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Applications can be defined in their CMake file as being `NOGUI`, but until 
> now this has had very limited effect. Especially on OS X, those applications 
> can still construct a minimal GUI and thus have "visual presence" in the Dock 
> and application switcher (and have a menubar as well).
> 
> This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for 
> those targets, and uses that token to set the default value for the 
> `GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.
> 
> This could potentially be combined on OS X with the CoreFoundation call that 
> turns a running application into an "agent" (see 
> https://git.reviewboard.kde.org/r/120354).
> 
> 
> Diffs
> -
> 
>   cmake/modules/KDE4Macros.cmake 073d726 
>   kdeui/kernel/kapplication.h fa2ab26 
>   kdeui/kernel/kapplication.cpp b093034 
>   kdeui/kernel/kuniqueapplication.h e05dcd7 
> 
> Diff: https://git.reviewboard.kde.org/r/120363/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14), rebuilt kdelibs, 
> kde-workspace, kde-runtime, kde-baseapps, kdepim-runtime and nepomuk-core.
> If the documentation I read is correct, the `GUIenabled` switch has no effect 
> on Linux, so this patch shouldn't have any either on that OS.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

2014-09-25 Thread Thomas Lübking


> On Sept. 24, 2014, 5:48 nachm., Thomas Lübking wrote:
> > I assume you'd be better off altering KMenu::addTitle() - or even patch Qt 
> > (QMenu on mach cannot deal w/ widget actions, at least if used on the 
> > global menubar)
> 
> René J.V. Bertin wrote:
> I agree totally, but for that
> 
> - I'd have to understand exactly what the addTitle does that makes Qt/Mac 
> crash
> - Ideally I'd also know how to determine if the menu is in the global 
> menubar or e.g. in a popup menu, where addTitle works perfectly fine. I think 
> we'd want to preserve that because popup menus follow the selected style and 
> not necessarily the OS X style.
> 
> There's also the point that the addTitle (and addSection, IIRC) in Qt5 
> don't crash. They have other issues (IIRC you get just a separator, not the 
> title text) but until now I've preferred to handle these crashes on a 
> case-by-case basis.
> 
> I admit, this RR was also made a bit with the idea of getting a 
> discussion going about this issue. ;)
> 
> Thomas Lübking wrote:
> Since KMenu is deprecated and the ::addTitle() implementation doesn't 
> differ in KF5, either the applications have simply been ported away from 
> KMenu or QWidgetAction was fixed in Qt5.
> 
> To know why exactly this crashes for you, i'd need to see a backtrace 
> (paste.kde.org) - Qt4 claimed QWidgetAction support on OSX' global menu - 
> with some caveats.
> If QMenu::menuAction() is in the action list of the global menu - 
> unfortunately, this menubar is parentless :-(
> Also there's no guarantee that this assignment won't change at some point 
> in the future to any direction.
> 
> > IIRC you get just a separator, not the title text
> 
> What basically means that the QWidget(Action) reparenting doesn't work at 
> all in Qt5 anymore (at best the linked out widget is just hidden)
> 
> 
> Disclaimer: I'm a bit biased here ;-)
> Imo using a QWidgetAction as title was a wrong design itfp - I proposed a 
> Qt4 patch to use a leading and entitled separator instead, but it was 
> rejected because not all styles did/do support texted separators. No idea 
> whether that patch was revived for Qt5, never tested. (And, tbh, I don't know 
> whether the native styles, ie. Win and Mac, support texted separators)
> 
> René J.V. Bertin wrote:
> backtrace: http://paste.kde.org/pvnu8pgui
> 
> If I recall correctly, Qt5.3's QMenu::addTitle and QMenu::addSection 
> indeed call for what I think you mean with texted separators. And OS X will 
> only render the separator for those. OS X 10.6 in any case, but I don't see 
> why that would have changed in later versions.

Thanks.
QMenu::addTitle() does not exist in 5.3 and ::setTitle() refers to the menubar 
item text.
::addSection() might work (if the building loop was reversed, making a 
separator as first element possible ;-)

On the crash:
It occurs because QWidgetPrivate::setGeometry_sys_helper() in qwidget_mac.mm is 
not aware that the widget it operates on is a toplevel widget (and has no 
parent)
This seems to be the "QMacNativeWidget(0);" "container" created in 
qmenu_mac.mm, QMenuPrivate::QMacMenuPrivate::addAction()

Why it doesn't figure so, I don't know, but assume that in

```cpp
bool QWidgetPrivate::isRealWindow() const
{
return q_func()->isWindow() && !topData()->embedded;
}
```

"topData()->embedded" will be true (so the return be false)


- Thomas


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


On Sept. 25, 2014, 2 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120355/
> ---
> 
> (Updated Sept. 25, 2014, 2 nachm.)
> 
> 
> Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt 
> KDE.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> Mac OS X cannot handle the formatting used for title menu items when it 
> applies to items in the toplevel menu bar. An application calling 
> KMenu::addTitle on such a menu item will crash immediately, somewhere deep in 
> Qt.
> 
> This patch works around that crash by emulating the addTitle effect.
> 
> Curiously, the addTitle call that causes the crash when clicking on the Help 
> menu concerns a submenu of an item of the Tools menu...
> 
> 
> Diffs
> -
> 
>   konq-plugins/uachanger/uachangerplugin.cpp 5e2d094 
> 
> Diff: https://git.reviewboard.kde.org/r/120355/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.6.8 with kdelibs 4.14.1
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread Thomas Lübking

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


Aside Thiagos concern, what actually causes the menubar/docker item stuff?

Quoting QApplication (4.8):
> On X11, the window system is initialized if GUIenabled is true. If GUIenabled 
> is false, the application does not connect to the X server. On Windows and 
> Mac OS, currently the window system is always initialized, regardless of the 
> value of GUIenabled. This may change in future versions of Qt.

So it's perhaps rather a statement in KApplicationPrivate::init() which causes 
this?
There aren't too many depending on GUIEnabled ... ;-)

- Thomas Lübking


On Sept. 25, 2014, 1:32 nachm., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120363/
> ---
> 
> (Updated Sept. 25, 2014, 1:32 nachm.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Applications can be defined in their CMake file as being `NOGUI`, but until 
> now this has had very limited effect. Especially on OS X, those applications 
> can still construct a minimal GUI and thus have "visual presence" in the Dock 
> and application switcher (and have a menubar as well).
> 
> This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for 
> those targets, and uses that token to set the default value for the 
> `GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.
> 
> This could potentially be combined on OS X with the CoreFoundation call that 
> turns a running application into an "agent" (see 
> https://git.reviewboard.kde.org/r/120354).
> 
> 
> Diffs
> -
> 
>   cmake/modules/KDE4Macros.cmake 073d726 
>   kdeui/kernel/kapplication.h fa2ab26 
>   kdeui/kernel/kapplication.cpp b093034 
>   kdeui/kernel/kuniqueapplication.h e05dcd7 
> 
> Diff: https://git.reviewboard.kde.org/r/120363/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14), rebuilt kdelibs, 
> kde-workspace, kde-runtime, kde-baseapps, kdepim-runtime and nepomuk-core.
> If the documentation I read is correct, the `GUIenabled` switch has no effect 
> on Linux, so this patch shouldn't have any either on that OS.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread Thiago Macieira
On Thursday 25 September 2014 13:32:41 René J.V. Bertin wrote:
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120363/
> ---
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Applications can be defined in their CMake file as being `NOGUI`, but until
> now this has had very limited effect. Especially on OS X, those
> applications can still construct a minimal GUI and thus have "visual
> presence" in the Dock and application switcher (and have a menubar as
> well).
> 
> This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for
> those targets, and uses that token to set the default value for the
> `GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.
> 
> This could potentially be combined on OS X with the CoreFoundation call that
> turns a running application into an "agent" (see
> https://git.reviewboard.kde.org/r/120354).

Note that this change can't apply to a Qt5-based build since QApplication no 
longer has that extra parameter.

Are we sure we want to add a new feature that is Qt4-only and has a short 
lifetime?
-- 
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: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin


> On Sept. 25, 2014, 5:58 p.m., Martin Gräßlin wrote:
> > kglobalaccel/main.cpp, lines 48-55
> > 
> >
> > now there are two deleted lines...

My bad, I could so hardly believe that one might want 2 empty lines there that 
your critique flipped sense in my mind ...


- René J.V.


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


On Sept. 25, 2014, 5:22 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 5:22 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 6:14 p.m.)


Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.


Repository: kde-runtime


Description
---

See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.

KDE helper applications that need to be able to present widgets or otherwise 
"talk with the GUI layer" require special attention on OS X, if one doesn't 
want them to appear in the Dock or task switcher nor present a bare-bones 
menubar when made active.

Applications that live in an app bundle can set LSUIElement="1" in their 
Info.plist to signal the window server that they're "agents" (and thus don't 
want the aforementioned visual presence). This feature is already in use (see 
Info.plis.template for apps like kded4 and kdeinit4, and the corresponding code 
in their respective CMake files).

kglobalaccel is a different case as it's built as a standard *n*x app 
(`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has no 
Info.plist. It is however possible to set the corresponding bit via 
CoreFoundation, and that's what this patch does.

Suggestion: a member function I'd tentatively call `appIsService` would be 
welcome, but one could also make this the default behaviour when starting a 
`GUIenabled=false` application on OS X.
That's actually the main reason for submitting this RR: see if we can come to a 
consensus if and how to use this new knowledge.


Diffs (updated)
-

  kglobalaccel/main.cpp 4d230b8 

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


Testing
---

On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).


Thanks,

René J.V. Bertin



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread Martin Gräßlin

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



kglobalaccel/main.cpp


now there are two deleted lines...


- Martin Gräßlin


On Sept. 25, 2014, 5:22 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 5:22 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 5:22 p.m.)


Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.


Repository: kde-runtime


Description
---

See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.

KDE helper applications that need to be able to present widgets or otherwise 
"talk with the GUI layer" require special attention on OS X, if one doesn't 
want them to appear in the Dock or task switcher nor present a bare-bones 
menubar when made active.

Applications that live in an app bundle can set LSUIElement="1" in their 
Info.plist to signal the window server that they're "agents" (and thus don't 
want the aforementioned visual presence). This feature is already in use (see 
Info.plis.template for apps like kded4 and kdeinit4, and the corresponding code 
in their respective CMake files).

kglobalaccel is a different case as it's built as a standard *n*x app 
(`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has no 
Info.plist. It is however possible to set the corresponding bit via 
CoreFoundation, and that's what this patch does.

Suggestion: a member function I'd tentatively call `appIsService` would be 
welcome, but one could also make this the default behaviour when starting a 
`GUIenabled=false` application on OS X.
That's actually the main reason for submitting this RR: see if we can come to a 
consensus if and how to use this new knowledge.


Diffs (updated)
-

  kglobalaccel/main.cpp 4d230b8 

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


Testing
---

On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).


Thanks,

René J.V. Bertin



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread Martin Gräßlin

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



kglobalaccel/main.cpp


looks like an unrelated code line deletion.


- Martin Gräßlin


On Sept. 25, 2014, 4:58 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 4:58 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin


> On Sept. 25, 2014, 4:10 p.m., Martin Gräßlin wrote:
> > kglobalaccel/main.cpp, line 28
> > 
> >
> > it's obvious that the include is needed, otherwise one wouldn't add it. 
> > I think that comment is not adding any information

As you wish. I added the comment so it'd have more chance to be removed when in 
some future version the CoreFoundation code is move somewhere upstream.


- René J.V.


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


On Sept. 25, 2014, 4:58 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 4:58 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 4:58 p.m.)


Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.


Changes
---

Address the issues Marin raised.


Repository: kde-runtime


Description
---

See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.

KDE helper applications that need to be able to present widgets or otherwise 
"talk with the GUI layer" require special attention on OS X, if one doesn't 
want them to appear in the Dock or task switcher nor present a bare-bones 
menubar when made active.

Applications that live in an app bundle can set LSUIElement="1" in their 
Info.plist to signal the window server that they're "agents" (and thus don't 
want the aforementioned visual presence). This feature is already in use (see 
Info.plis.template for apps like kded4 and kdeinit4, and the corresponding code 
in their respective CMake files).

kglobalaccel is a different case as it's built as a standard *n*x app 
(`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has no 
Info.plist. It is however possible to set the corresponding bit via 
CoreFoundation, and that's what this patch does.

Suggestion: a member function I'd tentatively call `appIsService` would be 
welcome, but one could also make this the default behaviour when starting a 
`GUIenabled=false` application on OS X.
That's actually the main reason for submitting this RR: see if we can come to a 
consensus if and how to use this new knowledge.


Diffs (updated)
-

  kglobalaccel/main.cpp 4d230b8 

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


Testing
---

On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).


Thanks,

René J.V. Bertin



Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread Martin Gräßlin

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



kglobalaccel/main.cpp


how long will the "just" be valid? Someone reading this code in lets say 
five years?

I would either remove the comment or make it a more generic comment (no I, 
no time reference).



kglobalaccel/main.cpp


it's obvious that the include is needed, otherwise one wouldn't add it. I 
think that comment is not adding any information


- Martin Gräßlin


On Sept. 25, 2014, 4:02 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120354/
> ---
> 
> (Updated Sept. 25, 2014, 4:02 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.
> 
> KDE helper applications that need to be able to present widgets or otherwise 
> "talk with the GUI layer" require special attention on OS X, if one doesn't 
> want them to appear in the Dock or task switcher nor present a bare-bones 
> menubar when made active.
> 
> Applications that live in an app bundle can set LSUIElement="1" in their 
> Info.plist to signal the window server that they're "agents" (and thus don't 
> want the aforementioned visual presence). This feature is already in use (see 
> Info.plis.template for apps like kded4 and kdeinit4, and the corresponding 
> code in their respective CMake files).
> 
> kglobalaccel is a different case as it's built as a standard *n*x app 
> (`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has 
> no Info.plist. It is however possible to set the corresponding bit via 
> CoreFoundation, and that's what this patch does.
> 
> Suggestion: a member function I'd tentatively call `appIsService` would be 
> welcome, but one could also make this the default behaviour when starting a 
> `GUIenabled=false` application on OS X.
> That's actually the main reason for submitting this RR: see if we can come to 
> a consensus if and how to use this new knowledge.
> 
> 
> Diffs
> -
> 
>   kglobalaccel/main.cpp 4d230b8 
> 
> Diff: https://git.reviewboard.kde.org/r/120354/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>



Re: Review Request 120195: [OS X] make sure the appropriate menu items get put in the Application menu's About and Preferences items

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 4:06 p.m.)


Review request for KDE Software on Mac OS X, KDevelop and Qt KDE.


Changes
---

Added qt-kde as the issue that lead to this RR is caused by a Qt "feature" that 
should really be under configurability control.


Repository: kdevplatform


Description
---

This is a complement to https://git.reviewboard.kde.org/r/120149/

OS X has a so-called Application menu, sitting in the MenuBar between the Apple 
(?) menu and the File menu, that has items (actions) such as About and 
Preferences, which are meant to play the respective roles for the application.

Qt4 for Mac uses `QAction::text()` based heuristics to decide which items get 
`PreferencesRole`, `AboutRole` etc. roles and thus are put in the Application 
menu. This works fine for applications that create the standard menu actions 
via KStandardAction. Applications that do not adhere to that scheme, or that 
create menu actions with matching names *before* the standard actions will end 
up having the wrong actions in the Application menu. For KDevelop, this means 
that the About menu item will invoke `About KDevelop Platform` and the 
Preferences item `Configure selection` (normally in the Project menu). The 
former misinterpretation isn't a huge deal, but launching a project's 
configure/cmake procedure (without any kind of confirmation asked) when you 
think you'll be opening a settings panel is more than just annoying.

The patch is simple: set the "incriminated" actions' `menuRole` to `NoRole` 
when they are created, on OS X. This prevents them from being put in the 
Application menu even when the patches from the above RR are not installed 
(though in that latter case the Preferences menu will probably invoke another 
unintended action).


Diffs
-

  shell/mainwindow_p.cpp b50c444 

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


Testing
---

On OS X 10.6.8 with KDE/MacPorts 4.12.5 and kdelibs git/master with all 
MacPorts and my patches applied. Other platforms will not be affected by this 
patch.

I have not yet looked into how this plays out on Qt5/KF5. The patch presented 
here should have the same effect if menu roles function the same way as in Qt4. 
If Qt5 uses comparable heuristics and `KAction` made the transition to KF5, the 
other kdelibs patches should port over easily.


Thanks,

René J.V. Bertin



Re: Review Request 120149: [OS X] improved menubar experience: protected Preferences menu and cleaner "system tray"

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 4:03 p.m.)


Review request for KDE Software on Mac OS X, kdelibs, KDEPIM, Qt KDE, Marco 
Martin, and Olivier Goffart.


Changes
---

Added qt-kde as the issues raised in this RR can only be addressed cleanly by 
modifications to Qt


Repository: kdelibs


Description
---

This review is for 2 sets of changes; an initial one to the way "system tray" 
are rendered, and a newer set that protects the Preferences menu from getting 
linked to any action with an appropriate title.

-- the system tray:
Until now, "system tray" menus had some rendering issues on Mac OS X:

- The menu title, the 1st menu item that on Linux shows the application name, 
remained empty
- Menu items that can (in principle, potentially) show an icon always showed 
the icon

Point 1 was resolved by emulating the Linux addTitle/setTitle action in 
`KStatusNotifierItemPrivate::init()` : the menu title is implemented as a 
deactive standard menuitem followed by a separator. This makes the item stand 
out on a GUI that doesn't support the kind of formatting in menus as used in 
the Linux implementation.

Point 2 was identified as a Qt issue: `isIconVisibleInMenu` is ignored for 
systray menus. It was resolved by adding `KMenu::addAction` methods that 
overload the ones from QMenu that were hitherto inherited unchanged by KMenu. 
The only different method is `addAction(QAction*)` which removes the icon from 
the `QAction` if `isIconVisibleInMenu()` is false. The other `addAction` 
methods are "overloaded with themselves" with `using QMenu::addAction;` in the 
header file.

-- the Preferences menu item
This is a menu item living in the Application menu, a menu that sits in the 
menubar between the Apple (?) menu and the File menu. This menu also contains 
the Quit command.
KDE and Qt applications typically do not set up their menus in this fashion, so 
Qt provides an automatic way to put relevant menu items (actions) in the 
Application menu, using Apple's naming. The algorithm is described under 
QMenuBar in the Qt documentation: for the Preferences action, it will consider 
any action that has a text containing `config`, `options`, `settings` or 
`preferences`, and put it under the Preferences label if its menu role is set 
to `heuristic` (which appears to be the default).
In practice, many applications provide a series of menu actions with names that 
trigger this method, and they do not always create their own 
preferences/settings/configuration menu first. Yet it is the first menu action 
that matches that will be installed under the Preferences menu, with the 
Command-, shortcut. A good example is KDevelop: it will have a Preferences menu 
that activates the `Configure Selection` action - which does not open a 
settings dialog but launches the configure or cmake procedure for the selected 
project ...

My proposed solution overrides this Qt behaviour. On OS X, the `KAction(const 
QString &text, QObject *parent)` constructor calls a modified (static) function 
`setTextWithCorrectMenuRole` which checks the text against the patterns Qt will 
consider for `PreferencesRole`. If it finds a match, it will force the role to 
`NoRole`, unless it is a perfect match with the standard KDE configuration 
action for the application (`"&Configuration appName..."`) in which case it 
sets the role to `PreferencesRole`. This latter consideration allows kdelibs to 
"catch" the configuration menu for applications like KMail, which appear not to 
be created using KStandardActions.
This approach can be extended to other menu actions that end up incorrectly in 
the OS X Application menu.

Applications that create menu actions using QAction or a different KAction 
constructor will see no change (and should use `setMenuRole` selectively on OS 
X).


Diffs
-

  kdeui/notifications/kstatusnotifieritem.cpp 1b15d40 
  kdeui/actions/kaction.cpp 9e8f7fb 

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


Testing
---

Testing was done with kdelibs git/master and KDE/MacPorts on OS X 10.6.8 . The 
modified code is in compile-time conditional blocks used only on OS X, so no 
regressions are to be expected on other platforms.

KF5 is not production ready on OS X, so I am not currently able to port these 
modifications beyond KDE4. However, I did see that Qt5 has a new approach to 
adding titles to menus, which can be described as a "labelled separator". 
Backporting that function from the Qt5 source to kdelibs gave menu items that 
had the separator but not the text (title) label. It is thus likely that some 
kind of emulation will also be required with KF5, on OS X.

I considered doing the addTitle/setTitle emulation in kmenu.cpp, but decided 
against that for now. Menu titles are rendered as under Linux i

Re: Review Request 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 4:02 p.m.)


Review request for KDE Software on Mac OS X, KDE Runtime, kdelibs, and Qt KDE.


Changes
---

added qt-kde for potentially adding a call/switch to Q(Core)Application, which 
would do the CoreFoundation "I'm an agent" magic.


Repository: kde-runtime


Description
---

See https://bugs.kde.org/show_bug.cgi?id=339333 for more detailed discussion.

KDE helper applications that need to be able to present widgets or otherwise 
"talk with the GUI layer" require special attention on OS X, if one doesn't 
want them to appear in the Dock or task switcher nor present a bare-bones 
menubar when made active.

Applications that live in an app bundle can set LSUIElement="1" in their 
Info.plist to signal the window server that they're "agents" (and thus don't 
want the aforementioned visual presence). This feature is already in use (see 
Info.plis.template for apps like kded4 and kdeinit4, and the corresponding code 
in their respective CMake files).

kglobalaccel is a different case as it's built as a standard *n*x app 
(`/opt/local/bin/kglobalaccel` in a standard MacPorts install) and thus has no 
Info.plist. It is however possible to set the corresponding bit via 
CoreFoundation, and that's what this patch does.

Suggestion: a member function I'd tentatively call `appIsService` would be 
welcome, but one could also make this the default behaviour when starting a 
`GUIenabled=false` application on OS X.
That's actually the main reason for submitting this RR: see if we can come to a 
consensus if and how to use this new knowledge.


Diffs
-

  kglobalaccel/main.cpp 4d230b8 

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


Testing
---

On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14).


Thanks,

René J.V. Bertin



Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 4 p.m.)


Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt KDE.


Changes
---

added the qt-kde group


Repository: kde-baseapps


Description
---

Mac OS X cannot handle the formatting used for title menu items when it applies 
to items in the toplevel menu bar. An application calling KMenu::addTitle on 
such a menu item will crash immediately, somewhere deep in Qt.

This patch works around that crash by emulating the addTitle effect.

Curiously, the addTitle call that causes the crash when clicking on the Help 
menu concerns a submenu of an item of the Tools menu...


Diffs
-

  konq-plugins/uachanger/uachangerplugin.cpp 5e2d094 

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


Testing
---

OS X 10.6.8 with kdelibs 4.14.1


Thanks,

René J.V. Bertin



Re: Review Request 120202: [OS X] improvements to the kwallet/OSX keychain integration

2014-09-25 Thread René J . V . Bertin

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

(Updated Sept. 25, 2014, 3:58 p.m.)


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


Changes
---

I have addressed the idleTimer's slot issue by making the WalletPrivate class 
inherit QObject in addition to QOSXKeychain.h . For that I've had to move it to 
a new headerfile, `kwallet_mac.h`, a header I could have created earlier given 
the complexity `kwallet_mac.cpp` has attained. In the end this I thought this 
was the cleanest solution.

Now that this is out of the way (I hope), I'd appreciate some feedback on the 2 
open questions:

1- what is missing from my DBus implementation that could explain why I see the 
slots and signals in qdbusviewer but calls sent to the slots never arrive in my 
code? Or rather, how do I get it to work?

2- how to complete the DBus-free wallet-user registry? The only thing missing 
is a method to share the structure in distributed memory without a central 
server. I'd need something like QSharedMemory with resizing capabilities.
Should I stop looking and share the reference to another QSharedMemory instance 
rather than share the registry's representation directly? A kind of shared 
handle (pointer to pointer, in old Apple speak from pre-MMU days).
The requirements are simple: each application having a Wallet open should be 
able to read the current registry contents ("user list"), and add or remove 
oneself to/from it.
All those operations can be performed on a copy freshly checked out of shared 
(and locked) memory but I fear it'd be rather delicate and race-condition 
prone. Each client will need to attach to the shared reference as well as the 
shared resource (to which that reference refers), and I think they'd all need 
to release the shared resource when the shared reference changes.

Any thoughts?

There was some demand from the kde-mac community to try and come up with an 
approach not requiring a central server (kwalletd), so I'd probably want to get 
approach 2 working even if we get approach 1 to function.


Repository: kdelibs


Description
---

I'm still working on (the KDE4-based version of) my OS X keychain backend for 
kwallet. I'm at a point where I think I can present a work-in-progress in an RR 
because at least one feature has been improved enough to be of interest for 
everyone, and also because I could use feedback on how to proceed.
I'm currently focussing on 2 settings that are configured in the kwallet KCM 
(SystemSettings), and for which I'm working on an implementation not requiring 
kwalletd and/or DBus.

- idle time closing of wallets. This feature was not supported in the commited 
version presented in https://git.reviewboard.kde.org/r/119838/ The present 
patch adds an idleTimer and a shared lastAccessTime member. The idleTimer is 
reset each time a client performs one of a series of actions that I count as 
wallet accesses, and before resetting I update the idle timeout value from 
KConfig. When the timer fires, the elapsed time is compared to the shared last 
access time, and if it is >= the timeout, the wallet is closed. This applies 
only to "KDE keychains", so keychains used by OS X applications should not be 
affected.

- "close when last application exits". This requires maintaining a "user list" 
which keeps track of what application has what wallet open. I've implemented an 
"internal" version of such a registry, mapping wallet name to application names 
and the list of wallets they have open (a list of wallet reference, pid per 
application name). The registry is functional, but I have not yet decided 
(read: figured out) how to make a distributed representation of it.

So the work-in-progress concerns the distributed user registry. The idea would 
be to maintain the registry in shared memory, meaning it'd be reset (= 
disappear) when the last application exits, contrary to a file which can go 
stale. This would be simple if QSharedMemory objects could be resized, but 
apparently they cannot, so I'll have to look at other solutions possibly 
involving OS X frameworks (NSData and it's non-objectiveC version CFDataRef or 
CFMutableDataRef might be candidates). Suggestions welcome.

Other work in progress concerns a less wheel-reinventing approach that builds 
on kwalletd and DBus. I don't see why the code used in `kwallet.cpp` wouldn't 
work, but I must still misunderstand its finer details. The present patch 
contains outcommented code that does indeed cause kwalletd to be launched and 
slots and signals to become visible e.g. in `qdbusviewer`. But they don't work, 
which in turn makes the whole kwallet layer dysfunctional. Here too feedback is 
welcome on how what I'm missing and/or how to get this to work.
Once kwalletd works, wallet idle timeout closing and closing when the last 
client exi

Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled

2014-09-25 Thread René J . V . Bertin

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

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


Repository: kdelibs


Description
---

Applications can be defined in their CMake file as being `NOGUI`, but until now 
this has had very limited effect. Especially on OS X, those applications can 
still construct a minimal GUI and thus have "visual presence" in the Dock and 
application switcher (and have a menubar as well).

This patch proposes to define a preprocessor token, `KDE_WITHOUT_GUI`, for 
those targets, and uses that token to set the default value for the 
`GUIenabled` option of the `KApplication` and `KUniqueApplication` classes.

This could potentially be combined on OS X with the CoreFoundation call that 
turns a running application into an "agent" (see 
https://git.reviewboard.kde.org/r/120354).


Diffs
-

  cmake/modules/KDE4Macros.cmake 073d726 
  kdeui/kernel/kapplication.h fa2ab26 
  kdeui/kernel/kapplication.cpp b093034 
  kdeui/kernel/kuniqueapplication.h e05dcd7 

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


Testing
---

On OS X 10.6.8 with kdelibs 4.14.1 (git/kde4.14), rebuilt kdelibs, 
kde-workspace, kde-runtime, kde-baseapps, kdepim-runtime and nepomuk-core.
If the documentation I read is correct, the `GUIenabled` switch has no effect 
on Linux, so this patch shouldn't have any either on that OS.


Thanks,

René J.V. Bertin