Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
On Friday 26 September 2014 07:05:09 Marko Käning wrote: > Hi Thiago, > > On 26 Sep 2014, at 07:01 , Thiago Macieira wrote: > > 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). > > did you miss our efforts on KF5 including setting up an OSX/CI system [1]? > > René has also produced patches for KF5 frameworks, which my OSX/CI system > could prove to be buildable already. :-) > > So, we’re aware of it, but René is up to now focusing more on cleaning up > KDE4, since we’re not yet there with any KF5 ports on MacPorts. But we’re > getting there these days. :-D You're missing my point. My point is that this problem requires a Qt 5 solution, since the proposed solution won't work there. That solution should be understood, if not developed, before this patch gets accepted. -- 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 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira wrote: > 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. Applying a solution to a single OS is inherently wrong, and is what has made building KDE 4 on OSX/Windows so difficult. Patches should *always* be upstreamed, especially when we are essentially the distributor (in essence we are carrying forks of our own projects due to these patches). > > 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). That will be addressed I suspect - I don't see why we can't improve things now for the applications we do ship. > -- > 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 > Regards, Ben
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> 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.
>
> 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
Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
On Freitag, 26. September 2014 10:41:39 CEST, Ben Cooksley wrote: Applying a solution to a single OS is inherently wrong On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira wrote: And still it needs to be studied for Qt5 Afaics there are two major issues with this patch (and actually they make everyone right in this thread ;-) 1. It's Qt4 only 2. It's the broadsword We can skip 1 (which is not "fixable", the API doesn't exist anymore) and should focus on 2 - and the foil may implicitly handle this as well ;-) It is too broadsword as, as mentioned by Thiago, the GUIEnabled parameter has several implications on all systems (let alone that the patch attempts to control it by the so far unrelated NOGUI cmake switch). Altering it (everywhere) to fix an issue with one system only, (which is also only indirectly caused by it) is dangerous (read: "wrong in an LTS") But limiting modifications to one system only is just as wrong, since it will diversify behavior, thus harden future maintainance. Luckily the behavior whether there's a menubar and stuff on OSX can be controlled by a QCoreApplication attribute that *is* OSX specific: Qt::AA_MacPluginApplication Stops the Qt mac application from doing specific initializations that do not necessarily make sense when using Qt to author a plugin. This includes avoiding loading our nib for the main menu and not taking possession of the native menu bar. When setting this attribute to true will also set the AA_DontUseNativeMenuBar attribute to true. So my very first attempt would be to set that (unconditionally, ie. on all systems - it should be idempotent outside OSX anyway) before instantiating the application in the main funcs of the problematic applications AND LEAVE THE GUIEnabled PARAMETER UNTOUCHED. Charmingly, such change would be Qt5 compatible =) Personally I doubt that this needs to or should be injected by a cmake parameter, but added directly (notably since this will only affect a fistful of applications in KDE anyway) Cheers, Thomas
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> 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 120376: drKonqi Fix Bug 337742 - Unable to send report: error code 410 from Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120376/#review67481 --- Hi Frédéric, As announced on KDE Core devel, in http://lists.kde.org/?l=kde-core-devel&m=141016488132293&w=2 about 3 weeks ago, I also am working on Dr Konqi. I am about to publish a general patch, which is aimed at the present problem posed by the change to tokens in Bugzilla https://bugs.kde.org/show_bug.cgi?id=337742, but also intends to avoid such problems in future and to be forward-portable to KF5. My approach is to check the version number of the Bugzilla software and to switch to whichever security method is appropriate for that version: cookies, tokens or passwords-only. Of course, my patch will implement tokens for the time being, but the idea is to switch automatically to passwords-only when the time comes, without any new release of KDE software being necessary. See http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN in the documentation for Bugzilla 4.5.5 (the next version), as opposed to 4.4.5 (the current version). Bugzilla 4.5.5 allows tokens, but I believe they will be discontinued at some stage, though I cannot put my finger on where I have seen that discussion. This should avoid users having to experience further bugs in Dr Konqi's connection to bugs.kde.org. My patch is also intended to be extendable, so that Dr Konqi can be updated and re-released, ahead of time, if any further feature change is announced by Bugzilla and could adversely affect Dr Konqi. - Ian Wadham On Sept. 26, 2014, 4:21 a.m., Frédéric Sheedy wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120376/ > --- > > (Updated Sept. 26, 2014, 4:21 a.m.) > > > 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 120354: [OS X] turn kglobalaccel into an "agent", removing it from Dock and application switcher
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120354/ --- (Updated Sept. 26, 2014, 12:13 p.m.) Status -- This change has been marked as submitted. 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
> 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...) > > 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 ... > > 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 ... Taking this back to RR: On Friday September 26 2014 13:33:20 Thomas Lübking wrote: > On Freitag, 26. September 2014 10:41:39 CEST, Ben Cooksley wrote: > > Applying a solution to a single OS is inherently wrong > > > On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira wrote: > >> And still it needs to be studied for Qt5 > > Afaics there are two major issues with this patch (and actually they make > everyone right in this thread ;-) > > 1. It's Qt4 only > 2. It's the broadsword > > We can skip 1 (which is not "fixable", the API doesn't exist anymore) and > should focus on 2 - and the foil may implicitly handle this as well ;-) You know 4.8.7 is in the making? ;) Also, concerning 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 That may be true for Qapplication, but KApplication does preciously little with GUIenabled. It's either ignored on X11, or only serves to determine whether the clipboard is initialised or not. > It is too broadsword as, as mentioned by Thiago, the GUIEnabled parameter has > several implications on all systems (let alone that the patch attempts to > control it by the so far unrelated NOGUI cmake switch). True, but what *does* the NOGUI switch do? Not much that I can see, until now. I'd still like to make a case that if there is such a switch, it'd best do what you'd expect it to do. It's unambiguous as can be, and removes the burden of having to remember the exact call and add it to code you're working on - actually, also the burden of knowing about the issue in the first place! This is the same argument a Qt5 dev gave me for their heuristics menurole guessing feature. > Luckily the behavior whether there's a menubar and stuff on OSX can be > controlled by a QCoreApplication attribute that *is* OSX specific: > >Qt::AA_MacPluginApplication [So I went on a little profiling spree :)] Indeed, but this is not exactly what `GUIenabled=false` leads to ultimately in Qt4, on OS X. `qt_appType` is set to `QApplication::Tty` which is in fact the default (at least in Qt4) instead of `QApplication::GuiClient`, which in turn sets the global bool variable `qt_is_gui_used=false`. `qt_is_gui_used` is read in the OS X `qt_init()` function (in qapplication_mac.mm), and controls a whole lot more than what `AA_MacPluginApplication` controls. Roughly speaking, one can resume that the window system isn't initialised *at all* when `qt_is_gui_used==false`. Note that `qt_init` reads out the `LSUIElement` flag (as well as `LSBackgroundOnly`) from the app's InfoDictionary (Info.plist or set programmatically). This is then used to call `TransformProcessType(&psn, kProcessTransformToForegroundApplication)`, or not. (I'm surprised to see that it'd be up to the application to take that Info.plist flag into account; I'd have guessed the window manager would take care of it.) *FYI*: In Qt 5.3.1, `qt_appType` is called `qapplication_type`, `qt_is_gui_used` exis
Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
On Friday 26 September 2014 11:08:23 René J.V. Bertin wrote: > >Patches should *always* be upstreamed, especially when we are > >essentially the distributor (in essence we are carrying forks of our > >own projects due to these patches). > > Heh, and when upstream means Qt, we'll probably have to maintain our own > local patches waiting and hoping they'll ever get incorporated upstream? > Remark, that's about the same situation as we (kde-mac) are currently in > w.r.t. KDE Why do you think those patches would not get into Qt? -- 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
> On sep. 26, 2014, 11:54 matin, Ian Wadham wrote: > > Hi Frédéric, > > > > As announced on KDE Core devel, in > > http://lists.kde.org/?l=kde-core-devel&m=141016488132293&w=2 about 3 weeks > > ago, I also am working on Dr Konqi. > > > > I am about to publish a general patch, which is aimed at the present > > problem posed by the change to tokens in Bugzilla > > https://bugs.kde.org/show_bug.cgi?id=337742, but also intends to avoid such > > problems in future and to be forward-portable to KF5. My approach is to > > check the version number of the Bugzilla software and to switch to > > whichever security method is appropriate for that version: cookies, tokens > > or passwords-only. > > > > Of course, my patch will implement tokens for the time being, but the idea > > is to switch automatically to passwords-only when the time comes, without > > any new release of KDE software being necessary. See > > http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN > > in the documentation for Bugzilla 4.5.5 (the next version), as opposed to > > 4.4.5 (the current version). Bugzilla 4.5.5 allows tokens, but I believe > > they will be discontinued at some stage, though I cannot put my finger on > > where I have seen that discussion. > > > > This should avoid users having to experience further bugs in Dr Konqi's > > connection to bugs.kde.org. My patch is also intended to be extendable, so > > that Dr Konqi can be updated and re-released, ahead of time, if any further > > feature change is announced by Bugzilla and could adversely affect Dr Konqi. Great, good news! My patch was only a quick fix to what you are doing. - Frédéric --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120376/#review67481 --- On sep. 26, 2014, 4:21 matin, Frédéric Sheedy wrote: > > --- > 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 > >
Re: Review Request 120376: drKonqi Fix Bug 337742 - Unable to send report: error code 410 from Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120376/ --- (Updated sep. 26, 2014, 3:28 après-midi) Status -- This change has been discarded. 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
> 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...) > > 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 ... > > 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. Bertin wrote: > Taking this back to RR: > > On Friday September 26 2014 13:33:20 Thomas Lübking wrote: > > On Freitag, 26. September 2014 10:41:39 CEST, Ben Cooksley wrote: > > > Applying a solution to a single OS is inherently wrong > > > > > On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira > wrote: > > >> And still it needs to be studied for Qt5 > > > > Afaics there are two major issues with this patch (and actually they > make everyone right in this thread ;-) > > > > 1. It's Qt4 only > > 2. It's the broadsword > > > > We can skip 1 (which is not "fixable", the API doesn't exist anymore) > and should focus on 2 - and the foil may implicitly handle this as well ;-) > > You know 4.8.7 is in the making? ;) > Also, concerning 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 > > That may be true for Qapplication, but KApplication does preciously > little with GUIenabled. It's either ignored on X11, or only serves to > determine whether the clipboard is initialised or not. > > > It is too broadsword as, as mentioned by Thiago, the GUIEnabled > parameter has several implications on all systems (let alone that the patch > attempts to control it by the so far unrelated NOGUI cmake switch). > > True, but what *does* the NOGUI switch do? Not much that I can see, until > now. I'd still like to make a case that if there is such a switch, it'd best > do what you'd expect it to do. It's unambiguous as can be, and removes the > burden of having to remember the exact call and add it to code you're working > on - actually, also the burden of knowing about the issue in the first place! > This is the same argument a Qt5 dev gave me for their heuristics menurole > guessing feature. > > > Luckily the behavior whether there's a menubar and stuff on OSX can be > controlled by a QCoreApplication attribute that *is* OSX specific: > > > >Qt::AA_MacPluginApplication > > [So I went on a little profiling spree :)] > > Indeed, but this is not exactly what `GUIenabled=false` leads to > ultimately in Qt4, on OS X. `qt_appType` is set to `QApplication::Tty` which > is in fact the default (at least in Qt4) instead of > `QApplication::GuiClient`, which in turn sets the global bool variable > `qt_is_gui_used=false`. > `qt_is_gui_used` is read in the OS X `qt_init()` function (in > qapplication_mac.mm), and controls a whole lot more than what > `AA_MacPluginApplication` controls. Roughly speaking, one can resume that the > window system isn't initialised *at all* when `qt_is_gui_used==false`. > Note that `qt_init` reads out the `LSUIElement` flag (as well as > `LSBackgroundOnly`) from the app's InfoDictionary (Info.plist or set > programmatically). This is then used to call `TransformProcessType(&psn, > kProcessTransformTo
Re: Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
> 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...) > > 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 ... > > 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. Bertin wrote: > Taking this back to RR: > > On Friday September 26 2014 13:33:20 Thomas Lübking wrote: > > On Freitag, 26. September 2014 10:41:39 CEST, Ben Cooksley wrote: > > > Applying a solution to a single OS is inherently wrong > > > > > On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira > wrote: > > >> And still it needs to be studied for Qt5 > > > > Afaics there are two major issues with this patch (and actually they > make everyone right in this thread ;-) > > > > 1. It's Qt4 only > > 2. It's the broadsword > > > > We can skip 1 (which is not "fixable", the API doesn't exist anymore) > and should focus on 2 - and the foil may implicitly handle this as well ;-) > > You know 4.8.7 is in the making? ;) > Also, concerning 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 > > That may be true for Qapplication, but KApplication does preciously > little with GUIenabled. It's either ignored on X11, or only serves to > determine whether the clipboard is initialised or not. > > > It is too broadsword as, as mentioned by Thiago, the GUIEnabled > parameter has several implications on all systems (let alone that the patch > attempts to control it by the so far unrelated NOGUI cmake switch). > > True, but what *does* the NOGUI switch do? Not much that I can see, until > now. I'd still like to make a case that if there is such a switch, it'd best > do what you'd expect it to do. It's unambiguous as can be, and removes the > burden of having to remember the exact call and add it to code you're working > on - actually, also the burden of knowing about the issue in the first place! > This is the same argument a Qt5 dev gave me for their heuristics menurole > guessing feature. > > > Luckily the behavior whether there's a menubar and stuff on OSX can be > controlled by a QCoreApplication attribute that *is* OSX specific: > > > >Qt::AA_MacPluginApplication > > [So I went on a little profiling spree :)] > > Indeed, but this is not exactly what `GUIenabled=false` leads to > ultimately in Qt4, on OS X. `qt_appType` is set to `QApplication::Tty` which > is in fact the default (at least in Qt4) instead of > `QApplication::GuiClient`, which in turn sets the global bool variable > `qt_is_gui_used=false`. > `qt_is_gui_used` is read in the OS X `qt_init()` function (in > qapplication_mac.mm), and controls a whole lot more than what > `AA_MacPluginApplication` controls. Roughly speaking, one can resume that the > window system isn't initialised *at all* when `qt_is_gui_used==false`. > Note that `qt_init` reads out the `LSUIElement` flag (as well as > `LSBackgroundOnly`) from the app's InfoDictionary (Info.plist or set > programmatically). This is then used to call `TransformProcessType(&psn, > kProcessTransformTo
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/ --- (Updated Sept. 26, 2014, 7:28 p.m.) Review request for KDE Base Apps, KDE Software on Mac OS X, kdelibs, and Qt KDE. Changes --- Indeed, adding a simple check against q->parentWidget() in QWidgetPrivate::setGeometry_sys_helper helps prevent the application crash when the `KMenu::addTitle` is called. That's good and well, but the end result is that the added title doesn't appear at all. Compare the 1st screenshot with the 2nd (in which the addTitle emulation under review here is active). 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 File Attachments (updated) patch for qwidget_mac.mm https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch with the Qt patch https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png with the addTitle emulation patch https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png Thanks, René J.V. Bertin
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- changing this patch to prevent just the call to invalidateBuffer_resizeHelper (and setting isResize=false when that function cannot be called) shows something in place of the menu's title exactly once. All other times the menu is opened, empty space is shown. - René J.V. Bertin On Sept. 26, 2014, 7:28 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. 26, 2014, 7:28 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 > 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 > > > File Attachments > > > patch for qwidget_mac.mm > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch > with the Qt patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png > with the addTitle emulation patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> On Sept. 26, 2014, 5:40 nachm., René J.V. Bertin wrote: > > changing this patch to prevent just the call to > > invalidateBuffer_resizeHelper (and setting isResize=false when that > > function cannot be called) shows something in place of the menu's title > > exactly once. All other times the menu is opened, empty space is shown. No idea why you want to manipulate the resize flag, try just: diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm index f58a755..d6a2741 100644 --- a/src/gui/kernel/qwidget_mac.mm +++ b/src/gui/kernel/qwidget_mac.mm @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, int y, int w, int h, bool isM setWSGeometry(false, oldRect); -if (isResize && QApplicationPrivate::graphicsSystem()) +if (isResize && QApplicationPrivate::graphicsSystem() && q->parentWidget()) invalidateBuffer_resizeHelper(oldp, olds); } - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- On Sept. 26, 2014, 5:28 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. 26, 2014, 5:28 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 > > > File Attachments > > > patch for qwidget_mac.mm > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch > with the Qt patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png > with the addTitle emulation patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> On Sept. 26, 2014, 5:40 nachm., René J.V. Bertin wrote: > > changing this patch to prevent just the call to > > invalidateBuffer_resizeHelper (and setting isResize=false when that > > function cannot be called) shows something in place of the menu's title > > exactly once. All other times the menu is opened, empty space is shown. > > Thomas Lübking wrote: > No idea why you want to manipulate the resize flag, try just: > > diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm > index f58a755..d6a2741 100644 > --- a/src/gui/kernel/qwidget_mac.mm > +++ b/src/gui/kernel/qwidget_mac.mm > @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, > int y, int w, int h, bool isM > > setWSGeometry(false, oldRect); > > -if (isResize && QApplicationPrivate::graphicsSystem()) > +if (isResize && QApplicationPrivate::graphicsSystem() && > q->parentWidget()) > invalidateBuffer_resizeHelper(oldp, olds); > } blast. ```diff diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm index f58a755..d6a2741 100644 --- a/src/gui/kernel/qwidget_mac.mm +++ b/src/gui/kernel/qwidget_mac.mm @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, int y, int w, int h, bool isM setWSGeometry(false, oldRect); -if (isResize && QApplicationPrivate::graphicsSystem()) +if (isResize && QApplicationPrivate::graphicsSystem() && q->parentWidget()) invalidateBuffer_resizeHelper(oldp, olds); } ``` - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- On Sept. 26, 2014, 5:28 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. 26, 2014, 5:28 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 > > > File Attachments > > > patch for qwidget_mac.mm > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch > with the Qt patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png > with the addTitle emulation patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> On Sept. 26, 2014, 7:40 p.m., René J.V. Bertin wrote: > > changing this patch to prevent just the call to > > invalidateBuffer_resizeHelper (and setting isResize=false when that > > function cannot be called) shows something in place of the menu's title > > exactly once. All other times the menu is opened, empty space is shown. > > Thomas Lübking wrote: > No idea why you want to manipulate the resize flag, try just: > > diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm > index f58a755..d6a2741 100644 > --- a/src/gui/kernel/qwidget_mac.mm > +++ b/src/gui/kernel/qwidget_mac.mm > @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, > int y, int w, int h, bool isM > > setWSGeometry(false, oldRect); > > -if (isResize && QApplicationPrivate::graphicsSystem()) > +if (isResize && QApplicationPrivate::graphicsSystem() && > q->parentWidget()) > invalidateBuffer_resizeHelper(oldp, olds); > } > > Thomas Lübking wrote: > blast. > > ```diff > diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm > index f58a755..d6a2741 100644 > --- a/src/gui/kernel/qwidget_mac.mm > +++ b/src/gui/kernel/qwidget_mac.mm > @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, > int y, int w, int h, bool isM > > setWSGeometry(false, oldRect); > > -if (isResize && QApplicationPrivate::graphicsSystem()) > +if (isResize && QApplicationPrivate::graphicsSystem() && > q->parentWidget()) > invalidateBuffer_resizeHelper(oldp, olds); > } > ``` let's say that I unset isResize so that the rest of the function can finish in a slightly more appropriate fashion. I don't think it'd do to let it behave as if the resize helper did its job when that function hasn't been called. - René J.V. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- On Sept. 26, 2014, 7:28 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. 26, 2014, 7:28 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 > 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 > > > File Attachments > > > patch for qwidget_mac.mm > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch > with the Qt patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png > with the addTitle emulation patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 120355: [OS X] prevent a crash when opening konqueror's Help menu
> On Sept. 26, 2014, 5:40 nachm., René J.V. Bertin wrote: > > changing this patch to prevent just the call to > > invalidateBuffer_resizeHelper (and setting isResize=false when that > > function cannot be called) shows something in place of the menu's title > > exactly once. All other times the menu is opened, empty space is shown. > > Thomas Lübking wrote: > No idea why you want to manipulate the resize flag, try just: > > diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm > index f58a755..d6a2741 100644 > --- a/src/gui/kernel/qwidget_mac.mm > +++ b/src/gui/kernel/qwidget_mac.mm > @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, > int y, int w, int h, bool isM > > setWSGeometry(false, oldRect); > > -if (isResize && QApplicationPrivate::graphicsSystem()) > +if (isResize && QApplicationPrivate::graphicsSystem() && > q->parentWidget()) > invalidateBuffer_resizeHelper(oldp, olds); > } > > Thomas Lübking wrote: > blast. > > ```diff > diff --git a/src/gui/kernel/qwidget_mac.mm b/src/gui/kernel/qwidget_mac.mm > index f58a755..d6a2741 100644 > --- a/src/gui/kernel/qwidget_mac.mm > +++ b/src/gui/kernel/qwidget_mac.mm > @@ -4619,7 +4619,7 @@ void QWidgetPrivate::setGeometry_sys_helper(int x, > int y, int w, int h, bool isM > > setWSGeometry(false, oldRect); > > -if (isResize && QApplicationPrivate::graphicsSystem()) > +if (isResize && QApplicationPrivate::graphicsSystem() && > q->parentWidget()) > invalidateBuffer_resizeHelper(oldp, olds); > } > ``` > > René J.V. Bertin wrote: > let's say that I unset isResize so that the rest of the function can > finish in a slightly more appropriate fashion. I don't think it'd do to let > it behave as if the resize helper did its job when that function hasn't been > called. It did the resize in a reasonable fashion - at "worst" i't required to follow the function with isRealWindow, ie. have qt_mac_update_sizer() called, but the resize event very most likely needs to be sent. - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120355/#review67495 --- On Sept. 26, 2014, 5:28 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. 26, 2014, 5:28 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 > > > File Attachments > > > patch for qwidget_mac.mm > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/b5c2dd92-33db-4225-9750-d10e13f0f835__prevent_addTitleRelated_crash.patch > with the Qt patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/96f4fbfa-854e-4596-9f5f-d82f98a06955__Screen_shot_2014-09-26_at_19.16.20.png > with the addTitle emulation patch > > https://git.reviewboard.kde.org/media/uploaded/files/2014/09/26/5ddf4a63-b3bb-415a-815a-c06eb7a5c7f2__Screen_shot_2014-09-26_at_19.19.40.png > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 120282: Fix use of qgetenv in kshorturifilter
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120282/ --- (Updated Sept. 26, 2014, 9 p.m.) Status -- This change has been marked as submitted. Review request for KDE Runtime. Repository: kde-runtime Description --- This is probably a leftover from the replacement of getenv with qgetenv. The current code uses the const char * cast of QByteArray which then is destroyed leaving an invalid pointer. Diffs - kurifilter-plugins/shorturi/kshorturifilter.cpp 6ba79e7437b5477f9e19a1aa3fa9cf71ac60ba1b Diff: https://git.reviewboard.kde.org/r/120282/diff/ Testing --- This patch is included in the Debian packages since 22-01-2014 Thanks, Maximiliano Curia
Re: Review Request 120376: drKonqi Fix Bug 337742 - Unable to send report: error code 410 from Bugzilla
> On Sept. 26, 2014, 11:54 a.m., Ian Wadham wrote: > > Hi Frédéric, > > > > As announced on KDE Core devel, in > > http://lists.kde.org/?l=kde-core-devel&m=141016488132293&w=2 about 3 weeks > > ago, I also am working on Dr Konqi. > > > > I am about to publish a general patch, which is aimed at the present > > problem posed by the change to tokens in Bugzilla > > https://bugs.kde.org/show_bug.cgi?id=337742, but also intends to avoid such > > problems in future and to be forward-portable to KF5. My approach is to > > check the version number of the Bugzilla software and to switch to > > whichever security method is appropriate for that version: cookies, tokens > > or passwords-only. > > > > Of course, my patch will implement tokens for the time being, but the idea > > is to switch automatically to passwords-only when the time comes, without > > any new release of KDE software being necessary. See > > http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN > > in the documentation for Bugzilla 4.5.5 (the next version), as opposed to > > 4.4.5 (the current version). Bugzilla 4.5.5 allows tokens, but I believe > > they will be discontinued at some stage, though I cannot put my finger on > > where I have seen that discussion. > > > > This should avoid users having to experience further bugs in Dr Konqi's > > connection to bugs.kde.org. My patch is also intended to be extendable, so > > that Dr Konqi can be updated and re-released, ahead of time, if any further > > feature change is announced by Bugzilla and could adversely affect Dr Konqi. > > Frédéric Sheedy wrote: > Great, good news! My patch was only a quick fix to what you are doing. I did not mean that you should drop what you are doing and discard this review and patch completely... :-) Instead, we should work together and each be aware of what the other is doing. Please revive your patch and this review. >From what I can tell, the patch (after review and shipping) will be good >immediately and at least until the version after Bugzilla 4.5.x. Also, your >patch has some improvements to the testing, which is important. And I think we >need to get a fix into the closing versions of KDE 4 ASAP (next deadline >Thursday, 9 October). My fixes will be more long-term and I am running short >of days to work on them, due to other commitments, and anyway they may take a >while to review... So please revive your review and patch, Frédéric. One immediate comment: I found that I could retrieve the token by using the tag "token", but I could not use "token" in the "args" map. I had to use "Bugzilla_token". I have no idea why that is. I am using an Apple OS X platform, but that should not make a difference to a web dialog. - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120376/#review67481 --- On Sept. 26, 2014, 3:28 p.m., Frédéric Sheedy wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120376/ > --- > > (Updated Sept. 26, 2014, 3:28 p.m.) > > > 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 > >
