Kirigami 1.1 / QML problem (Subsurface-mobile)

2017-03-19 Thread Dirk Hohndel

Hi there,

After taking a pause for a while, I tried to address a couple of the bug 
reports for Subsurface-mobile on Android and iOS.

Right now we are building with Kirigami 1.1 and Qt 5.7.1. If we have to upgrade 
to Kirigami 2.0 and/or Qt 5.8 in order to make it easier for people to help us, 
please let me know... I tried with Qt 5.8.0 on the desktop and the two problems 
are still there. But I haven't tried Kirigami 2.0.

So what's the issue? Quick background: Subsurface-mobile shows a dive list as 
it's first page - that's a vertical ListView that shows a couple of lines of 
data for each dive through a fairly simple delegate. The data comes from a 
model that is implemented as a QSortFilterProxyModel in C++.
When you tap on one of the delegates, a new page is added which shows more 
information about the dive (most of that's in QML, but one pixmap with a 
visualization of the dive is rendered in C++). In order to be able to switch 
between dives by flicking that detailed view to the left or right, that "detail 
page" is implemented as another ListView using the same model, only with 
horizontal orientation. There's an "onCurrentIndexChanged" handler that keeps 
the outer and inner ListView in sync. Or at least that's supposed to do so. 
Lots and lots of debug prints seem to indicate that that part is working fine.

Anyway, same model should imply that the same index means the same dive, so 
this should all work, right?

We now run into two problems:

a) under some circumstances (and we can reproduce this, but it requires the 
full app, we don't have an "easy to build test case to show the problem", 
sadly) when you tap on the dive with index N in the first ListView (the 
vertical dive list), the dive that is shown in the details view is actually 
dive N+1. I have spent several days trying to figure out why - no clue. The 
worst part is that in many scenarios it actually works correctly.

b) under some circumstances (one is the case above, tap on N, it shows details 
for dive N+1, now in the vertical list tap on dive N again -- but there are 
other ways to get to this situation), things go crazy and the app will render 
ALL of the dives in the horizontal ListView (as shown by log message in the 
onCompleted signal handler for the delegate component) in the background. 
Depending on how many dives you have in the list, this will either hang the app 
for a considerable amount of time (tens of seconds) and then result in the 
correct page being shown, or it will crash the app, running out of memory.

I understand that asking for help saying "hey, there is this massively painful 
to build app (at least we now have this fully scripted, so getting an Android 
apk or the Subsurface-mobile version for a Linux desktop is no longer quite as 
difficult - at this point it mostly takes a lot of time), can you help fix our 
bug" isn't really a great request. If anyone wants to take a look (both Marco 
and Sebas have contributed tremendously to Subsurface-mobile in the past, 
without their help we would never have gotten as far as we are), of course we'd 
appreciate it. But what I really was hoping for was someone saying "Oh, right, 
I've seen something like this, have you tried `.'"

If someone wants to try reproduce this, I'm happy to send information on how to 
get started building Subsurface-mobile. I can also point you to existing 
binaries in the app stores (Android, iOS) and sample data in order to reproduce 
the problem in case you want to see this in action.

And as I said above, if you'd really rather have me update to Kirigami 2.0, I 
can make that higher priority and come back once that's done... I tried right 
around the time Kirigami 2 was released and had a rather frustrating time and 
kinda walked away from our mobile app in anger for a while... :-)

Either way, thanks for reading

/D



D5109: [Kicker/App Entry] Try QIcon with path if no theme icon is found

2017-03-19 Thread Kai Uwe Broulik
broulik added a comment.


  What's super weird about this one is that `QIcon::fromTheme` internally 
actually does
  
if (QDir::isAbsolutePath(name)) {
return QIcon(name);
}
  
  yet only with this patch I get an icon for `.svg` icons with an absolute path 
I configured. Maybe `icon.availableSizes()` is empty for svg images and thus 
the previously used fallback argument of `fromTheme` is always chosen.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D5109

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol


D5109: [Kicker/App Entry] Try QIcon with path if no theme icon is found

2017-03-19 Thread Kai Uwe Broulik
broulik created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  Applications can specify an absolute file path for an icon which won't work 
with QIcon::fromTheme.
  If a theme icon isn't find, first try to load it using an absolute path 
before falling back to an unknown icon.
  
  BUG: 365131

TEST PLAN
  5.9 branch?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D5109

AFFECTED FILES
  applets/kicker/plugin/appentry.cpp

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol


Re: Re: DrKonqi dependencies

2017-03-19 Thread Andreas Sturmlechner
On Sunday, 19 March 2017 at 14:19, Kai Uwe Broulik wrote:
> I'd be fine with splitting drkonqi into its own repository, René recently
> brought this up as well.

Since I was pondering the idea anyway, might as well attach my patch to build 
drkonqi standalone from its subdir.
commit 1ee858f194fd2c8dcb2cbd3f436fffe7207ba0cc
Author: Andreas Sturmlechner 
Date:   Sun Mar 19 12:54:34 2017 +0100

drkonqi: Standalone sub-project

diff --git a/drkonqi/CMakeLists.txt b/drkonqi/CMakeLists.txt
index cdae1d49..3923a8f4 100644
--- a/drkonqi/CMakeLists.txt
+++ b/drkonqi/CMakeLists.txt
@@ -1,5 +1,68 @@
+project(drkonqi)
+
 include (CheckFunctionExists)
 
+if(${CMAKE_SOURCE_DIR} STREQUAL ${drkonqi_SOURCE_DIR})
+set(PROJECT_VERSION "5.9.90")
+set(PROJECT_VERSION_MAJOR 5)
+
+cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR)
+
+set(QT_MIN_VERSION "5.7.0")
+set(KF5_MIN_VERSION "5.29.0")
+find_package(Qt5 ${QT_MIN_VERSION} CONFIG REQUIRED COMPONENTS DBus Gui Widgets Xml)
+find_package(ECM 1.8.0 REQUIRED NO_MODULE)
+set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH})
+
+include(KDEInstallDirs)
+include(KDECMakeSettings)
+include(KDECompilerSettings NO_POLICY_SCOPE)
+include(ECMPackageConfigHelpers)
+include(ECMMarkNonGuiExecutable)
+include(CMakePackageConfigHelpers)
+include(WriteBasicConfigVersionFile)
+include(CheckIncludeFiles)
+include(FeatureSummary)
+include(ECMOptionalAddSubdirectory)
+include(ECMQtDeclareLoggingCategory)
+include(KDEPackageAppTemplates)
+include(ECMMarkAsTest)
+
+find_package(KF5 ${KF5_MIN_VERSION} REQUIRED COMPONENTS
+Completion
+Config
+ConfigWidgets
+CoreAddons
+Crash
+I18n
+IdleTime
+JobWidgets
+KIO
+Notifications
+Service
+Wallet
+WidgetsAddons
+)
+find_package(KF5XmlRpcClient REQUIRED)
+
+find_package(X11)
+set_package_properties(X11 PROPERTIES DESCRIPTION "X11 libraries"
+URL "http://www.x.org;
+TYPE OPTIONAL
+PURPOSE "Required for X11 support")
+
+if(X11_FOUND)
+find_package(Qt5 ${QT_MIN_VERSION} CONFIG REQUIRED COMPONENTS X11Extras)
+set(HAVE_X11 1)
+endif()
+
+if(BUILD_TESTING)
+find_package(Qt5 ${QT_MIN_VERSION} CONFIG REQUIRED COMPONENTS Concurrent Test)
+endif()
+
+configure_file(../config-X11.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-X11.h)
+endif()
+
 check_function_exists("strsignal" HAVE_STRSIGNAL)
 check_function_exists("uname" HAVE_UNAME)
 
@@ -114,3 +177,7 @@ if (HAVE_X11)
 endif()
 
 install(TARGETS drkonqi DESTINATION ${KDE_INSTALL_LIBEXECDIR})
+
+if ("${CMAKE_BINARY_DIR}" STREQUAL "${CMAKE_CURRENT_BINARY_DIR}")
+feature_summary(WHAT ALL FATAL_ON_MISSING_REQUIRED_PACKAGES)
+endif()


Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread René J . V . Bertin
Martin Gräßlin wrote:

Hi,

> KWin is only involved starting in point 4. 1-3 are in the application.

I think I'm beginning to grasp what's going on, thanks.
QtCurve has a feature that allows to specify a modification of the menu 
background shade, which I have to set to 100% lighter. That would explain the 
symptom I'm seeing if that percentage is calculated with respect to the 
application palette instead of to the widget palette, and if memory serves me 
well that is what happens.

Bummer. The only proper way I see around this is to detect the useractions menu 
inside the style (how?!) and compare the base ("unlightened/darkened") 
background colour to the widget's actual background colour, and use the 
lightened colour only when that comparison finds no difference.
As an alternative it might be possible and simpler (but overkill?) to compare 
the menu's palette to the application palette and use the former if different.

R.



D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin updated this revision to Diff 12618.
graesslin added a comment.


  Adressed David's comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5106?vs=12614=12618

BRANCH
  screenedge-touch

REVISION DETAIL
  https://phabricator.kde.org/D5106

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/test_gestures.cpp
  gestures.cpp
  gestures.h
  input.cpp
  screenedge.cpp
  screenedge.h

To: graesslin, #kwin, #plasma_on_wayland
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, eliasp, sebas, apol


D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin marked 4 inline comments as done.
graesslin added inline comments.

INLINE COMMENTS

> davidedmundson wrote in screenedge.cpp:473
> Note that the only subclass of Edge doesn't call the super class for any of 
> the virtual methods: doGeom/activate/deactivate.
> 
> To fit the current pattern this should be in setGeometry / reserve / 
> unreserve respectively
> 
> (and yes, I know the current only subclass of Edge isn't relevant in this 
> case)

Not sure about this one. I thought of just calling the parent method when 
adding it on X11. Also long term I want to get rid of the WindowBasedEdge and 
just use XInput2.

REVISION DETAIL
  https://phabricator.kde.org/D5106

To: graesslin, #kwin, #plasma_on_wayland
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, eliasp, sebas, apol


D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin added inline comments.

INLINE COMMENTS

> davidedmundson wrote in screenedge.cpp:76
> who owns SwipeGesture?

good catch, that should be new SwipeGesture(this)

REVISION DETAIL
  https://phabricator.kde.org/D5106

To: graesslin, #kwin, #plasma_on_wayland
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, eliasp, sebas, apol


Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread Martin Gräßlin

Am 2017-03-19 17:02, schrieb René J. V.  Bertin:

Martin Gräßlin wrote:

Thanks for the info.


I need to explain the whole interaction:
1. Applications can use KColorSchemeManager to set a color scheme.
2. This gets installed as a dynamic property in the QGuiApplication


This happens in KColorSchemeManager or at a lower level?


The dynamic property gets set by KColorSchemeManager.




3. plasma-integration reads this dynamic property and installs an X
property on each created window
4. KWin reads the property and creates a QPalette from it
5. The palette gets stored for each window and is passed to the
decoration and to the useractions context menu


Reading this on is tempted to think that as long as a style handles a 
widget's
QPalette correctly there should be no issue like what I'm seeing. And 
QtCurve
does handle palettes correctly for contextmenus, except in KWin5 when 
it's not

using the standard palette.

It'll probably be easier to understand why the contextmenu isn't 
updated
correctly when the application changes its palette rather than via a 
special

rule.


The code in question is in useractions.cpp in multiple places, just
search for setPalette.


Is m_client.data()->palette() how useractions.cpp accesses the dynamic 
property

mentioned under point 2)?


KWin is only involved starting in point 4. 1-3 are in the application.

Cheers
Martin


D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread David Edmundson
davidedmundson added a comment.


  mostly looks fine. Only one major comment about the return after handling 
client edges.

INLINE COMMENTS

> gestures.cpp:61
> +case Direction::Right:
> +return std::min(std::abs(delta.width()) / 
> std::abs(m_minimumDelta.width()), 1.0);
> +default:

having negatives in minimumDelta would make no sense in the first place..

> screenedge.cpp:76
>  , m_client(nullptr)
> +, m_gesture(new SwipeGesture)
>  {

who owns SwipeGesture?

> screenedge.cpp:84
> +m_client->showOnScreenEdge();
> +unreserve();
> +}

equivalent ::handle() code returns after this.

> screenedge.cpp:92
> +connect(m_gesture, ::cancelled, this, 
> ::stopApproaching);
> +connect(m_gesture, ::triggered, this, 
> ::stopApproaching);
> +connect(m_gesture, ::progress, this,

maybe we want to do this inside the triggered lambda above, otherwise we call 
stopApproaching, before handling the trigger which is queued.

If our wayland side ever gets a doStopApproaching, it might result in being a 
bit out of order

> screenedge.cpp:473
>  {
> +if (isScreenEdge() && !m_edges->isDesktopSwitching()) {
> +m_edges->gestureRecognizer()->registerGesture(m_gesture);

Note that the only subclass of Edge doesn't call the super class for any of the 
virtual methods: doGeom/activate/deactivate.

To fit the current pattern this should be in setGeometry / reserve / unreserve 
respectively

(and yes, I know the current only subclass of Edge isn't relevant in this case)

REVISION DETAIL
  https://phabricator.kde.org/D5106

To: graesslin, #kwin, #plasma_on_wayland
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, eliasp, sebas, apol


Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread René J . V . Bertin
Martin Gräßlin wrote:

Thanks for the info.

> I need to explain the whole interaction:
> 1. Applications can use KColorSchemeManager to set a color scheme.
> 2. This gets installed as a dynamic property in the QGuiApplication

This happens in KColorSchemeManager or at a lower level?

> 3. plasma-integration reads this dynamic property and installs an X
> property on each created window
> 4. KWin reads the property and creates a QPalette from it
> 5. The palette gets stored for each window and is passed to the
> decoration and to the useractions context menu

Reading this on is tempted to think that as long as a style handles a widget's 
QPalette correctly there should be no issue like what I'm seeing. And QtCurve 
does handle palettes correctly for contextmenus, except in KWin5 when it's not 
using the standard palette.

It'll probably be easier to understand why the contextmenu isn't updated 
correctly when the application changes its palette rather than via a special 
rule.

> The code in question is in useractions.cpp in multiple places, just
> search for setPalette.

Is m_client.data()->palette() how useractions.cpp accesses the dynamic property 
mentioned under point 2)?

I'm currently updating QtCurve so it inherits KStyle instead of QCommonStyle . 
No idea yet if that's related but it should probably be done anyway.

R.



Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread Martin Gräßlin

Am 2017-03-19 13:27, schrieb René J. V.  Bertin:
subsidiary question: QtCurve connects to certain signals that I think 
come from

KWin:

bus.connect("org.kde.kwin", "/KWin", "org.kde.KWin", 
"compositingToggled",

this, SLOT(compositingToggled()));


This connect is broken. KWin no longer emits the signal on /KWin and 
neither in the org.kde.KWin interface.


If you need this I suggest to replace it by: 
KWindowSystem::compositingChanged signal




QString arg0 = qApp? qApp->arguments()[0] : QString();
if (!qApp || (arg0 != "kwin" && arg0 != "kwin_x11" && arg0 !=
"kwin_wayland")) {
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"themeChanged", this, SLOT(borderSizesChanged()));
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"borderSizesChanged", this, 
SLOT(borderSizesChanged()));

if (opts.menubarHiding & HIDE_KWIN)
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"toggleMenuBar",
this, SLOT(toggleMenuBar(unsigned int)));


KWin no longer takes org.kde.kwin, but org.kde.KWin. So the connects are 
100 % broken.




The tweak to omit these connects in KWin itself comes from a Debian 
patch, I
just added the connect to themeChanged() myself after noticing in the 
Aurorae

theme sources shipping with KWin 5.9.3 .

Are those connects still relevant, and should they indeed be excluded 
for KWin

itself?


Whether or not I cannot tell you as I have no idea what QtCurve used to 
do there. KWin does not have any /QtCurve installed. That's a question 
you would have to pass to QtCurve developers.


Cheers
Martin


Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread Martin Gräßlin

Am 2017-03-19 11:08, schrieb René J.V. Bertin:

Hi,

I've mentioned an issue where the window context menus drawn by
QtCurve do not adopt a new colour palette correctly when I change a
window's or application's palette from the session default.

This happens both when I use a window or app KWin rule and when I
change the application palette in an application (e.g. KDevelop). In
the latter case the window titlebar colour changes to follow the
selected palette, so I deduce that KWin detects the change and applies
a dynamic form of an app-specific palette rule.

What happens is that the contextmenu background is not changed, only
the item (QAction) text is, but only in the contextmenus shown by
KWin. When I change the entire application palette the application's
contextmenus render correctly.

This appears to be due to some interplay between KWin and QtCurve
which I'd like to address.

3 questions that seem relevant:
- does KWin indeed handle an in-app palette change as a dynamic
app-specific palette rule?
- where in KWin's source is this rule implemented (the actual palette 
change)?

- is there anything that makes KWin's window contextmenus different
from other context menus in other applications?


I need to explain the whole interaction:
1. Applications can use KColorSchemeManager to set a color scheme.
2. This gets installed as a dynamic property in the QGuiApplication
3. plasma-integration reads this dynamic property and installs an X 
property on each created window

4. KWin reads the property and creates a QPalette from it
5. The palette gets stored for each window and is passed to the 
decoration and to the useractions context menu


The steps 1-3 can be replaced by a window specific rule.

The code in question is in useractions.cpp in multiple places, just 
search for setPalette.


Cheers
Martin


Re: DrKonqi dependencies

2017-03-19 Thread Martin Gräßlin

Am 2017-03-19 14:19, schrieb Kai Uwe Broulik:

I'd be fine with splitting drkonqi into its own repository, René
recently brought this up as well.


Ideally it would be part of KF5, e.g. a kcrash-runtime tier-3 repo.



It might need some adjustments to the environment it runs in, though.
In Plasma we show an SNI and an interactive notification. Other
platforms might not support interactive notifications.


*shrug* can be done by those who want to use it outside of Plasma ;-)

Cheers
Martin


Re: DrKonqi dependencies

2017-03-19 Thread Kai Uwe Broulik
I'd be fine with splitting drkonqi into its own repository, René recently 
brought this up as well.

It might need some adjustments to the environment it runs in, though. In Plasma 
we show an SNI and an interactive notification. Other platforms might not 
support interactive notifications.




Re: DrKonqi dependencies

2017-03-19 Thread Andreas Sturmlechner
On 19.03.2017 11:56, Christoph Feck wrote:
> Using openSUSE packages here, installing drkonqi does not drag in the 
> Plasma workspace, only some KF5 frameworks. So the splitting can also be 
> done by the distribution.

Them fixing it in packaging does not make it right though. Everone building 
drkonqi still has to go through all of plasma-workspace or hack around it. 
Every (?) application linking against KCrash does profit from drkonqi - to me 
this is very similar to khelpcenter, or speaking of lib-app pairs, KF5Wallet 
and kwalletmanager. The question imo is if drkonqi should rather be in 
Applications or be kept as part of Plasma, just standalone.

Regards,
Andreas



D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin added a dependency: D5097: Add support for global touchpad swipe 
gestures.

REVISION DETAIL
  https://phabricator.kde.org/D5106

To: graesslin, #kwin, #plasma_on_wayland
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol


D5106: Add support for activating screenedges through touch swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin created this revision.
Restricted Application added a subscriber: plasma-devel.
Restricted Application added a project: Plasma on Wayland.

REVISION SUMMARY
  Each Edge creates a SwipeGesture for touch activation. The swipe needs to
  be a single finger starting from the edge into the screen for at least
  20 %. The SwipeGesture and GestureRecognizer is extended to support the
  use cases of the touch screen edge swipe.
  
  New features supported by the gesture system are:
  
  - minimum and maximum position
  - a minimum delta for the swipe
  - progress signal based on the minimum delta
  - starting a swipe with a start point
  
  The Edge has the progress signal connected to its approach signal, thus
  visual feedback is provided through the screen edge effect.
  
  The screen edge system supports touch only for the edges (corners are
  too difficult to activate on touch screens). At the moment the following
  features are supported:
  
  - screen edge show/raise of windows (e.g. auto hidden panels)
  - trigger the configured action
  - trigger the configured callback function (e.g. script)
  
  In future it might make sense to add a touch specific configuration
  action to support different actions for screen edges activated by mouse
  and touch.
  
  BUG: 370323

TEST PLAN
  configured a screen edge and triggered through touch,
  added an auto-hiding panel and triggered through touch

BRANCH
  screenedge-touch

REVISION DETAIL
  https://phabricator.kde.org/D5106

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/test_gestures.cpp
  gestures.cpp
  gestures.h
  input.cpp
  screenedge.cpp
  screenedge.h

To: graesslin, #kwin, #plasma_on_wayland
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol


D5097: Add support for global touchpad swipe gestures

2017-03-19 Thread Martin Gräßlin
graesslin added a dependent revision: D5106: Add support for activating 
screenedges through touch swipe gestures.

REVISION DETAIL
  https://phabricator.kde.org/D5097

To: graesslin, #kwin, #plasma_on_wayland
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
eliasp, sebas, apol


Re: KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread René J . V . Bertin
subsidiary question: QtCurve connects to certain signals that I think come from 
KWin:

bus.connect("org.kde.kwin", "/KWin", "org.kde.KWin", "compositingToggled",
this, SLOT(compositingToggled()));

QString arg0 = qApp? qApp->arguments()[0] : QString();
if (!qApp || (arg0 != "kwin" && arg0 != "kwin_x11" && arg0 != 
"kwin_wayland")) {
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"themeChanged", this, SLOT(borderSizesChanged()));
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"borderSizesChanged", this, SLOT(borderSizesChanged()));
if (opts.menubarHiding & HIDE_KWIN)
bus.connect("org.kde.kwin", "/QtCurve", "org.kde.QtCurve",
"toggleMenuBar",
this, SLOT(toggleMenuBar(unsigned int)));

The tweak to omit these connects in KWin itself comes from a Debian patch, I 
just added the connect to themeChanged() myself after noticing in the Aurorae 
theme sources shipping with KWin 5.9.3 .

Are those connects still relevant, and should they indeed be excluded for KWin 
itself?

R.



Re: DrKonqi dependencies

2017-03-19 Thread Christoph Feck

On 19.03.2017 11:52, Jasem Mutlaq wrote:

Problem is DrKonqi is now part of plasma-workspace. This makes it a *HUGE*
dependency just for crash reporting. I obviously cannot ask non-KDE users
to install plasma-workspace _just_ to have crash reporting capabilities.


Using openSUSE packages here, installing drkonqi does not drag in the 
Plasma workspace, only some KF5 frameworks. So the splitting can also be 
done by the distribution.


Review Request 130030: taskmanager : Implement window group dragging

2017-03-19 Thread Yoann Laissus

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

It can be dropped to other virtual desktops, for instance.


Diffs
-

  libtaskmanager/taskgroupingproxymodel.cpp 
ae1d6cfb7857199c2bc7e6aa515639add2d50a14 

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


Testing
---

Tested with drops on other virtual desktops.
Only tested on X11. I'm not sure it can work on Wayland as the drag handling 
seems to be very X11 specific.


Thanks,

Yoann Laissus



D5105: Check for Mycroft Service Status in Background

2017-03-19 Thread Aditya Mehra
This revision was automatically updated to reflect the committed changes.
Closed by commit R846:1ceed0475e66: Check for Mycroft Service Status in 
Background (authored by Aiix).

REPOSITORY
  R846 Mycroft Plasma integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5105?vs=12611=12612

REVISION DETAIL
  https://phabricator.kde.org/D5105

AFFECTED FILES
  .arcconfig
  plasmoid/contents/ui/main.qml

To: Aiix
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol


D5105: Check for Mycroft Service Status in Background

2017-03-19 Thread Aditya Mehra
Aiix created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  Ref https://phabricator.kde.org/T5704
  Added new statussocket listner which on connect will notify message socket to 
activate or on statussocket error will keep message socket disabled

TEST PLAN
  Mycroft running in background, when plasmashell crashes or on locking screen

REPOSITORY
  R846 Mycroft Plasma integration

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5105

AFFECTED FILES
  .arcconfig
  plasmoid/contents/ui/main.qml

To: Aiix
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol


DrKonqi dependencies

2017-03-19 Thread Jasem Mutlaq
Hello,

I have the following issue. We have KStars, part of KDE Applications, which
is used across many desktop environments. I recently wanted to see if
DrKonqi could be used to catch crashes and report them back to bugs.kde.org

Problem is DrKonqi is now part of plasma-workspace. This makes it a *HUGE*
dependency just for crash reporting. I obviously cannot ask non-KDE users
to install plasma-workspace _just_ to have crash reporting capabilities.

So I understand DrKonqi has a lot of dependencies, but what is the proper
solution to this? Can be split into its own thing?

-- 
Best Regards,
Jasem Mutlaq


KWin contextmenus usability and window/app specific palettes

2017-03-19 Thread René J . V . Bertin
Hi,

I've mentioned an issue where the window context menus drawn by QtCurve do not 
adopt a new colour palette correctly when I change a window's or application's 
palette from the session default.

This happens both when I use a window or app KWin rule and when I change the 
application palette in an application (e.g. KDevelop). In the latter case the 
window titlebar colour changes to follow the selected palette, so I deduce that 
KWin detects the change and applies a dynamic form of an app-specific palette 
rule.

What happens is that the contextmenu background is not changed, only the item 
(QAction) text is, but only in the contextmenus shown by KWin. When I change 
the entire application palette the application's contextmenus render correctly.

This appears to be due to some interplay between KWin and QtCurve which I'd 
like to address.

3 questions that seem relevant:
- does KWin indeed handle an in-app palette change as a dynamic app-specific 
palette rule?
- where in KWin's source is this rule implemented (the actual palette change)?
- is there anything that makes KWin's window contextmenus different from other 
context menus in other applications?

Thanks,
René


D4799: Delay notifications until desktop session has loaded

2017-03-19 Thread Valerio Pilo
vpilo added a comment.


  I pushed https://phabricator.kde.org/D5012; this rev is now testable.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D4799

To: vpilo, #plasma, #plasma_workspaces, davidedmundson, dfaure, broulik, 
graesslin, mck182
Cc: plasma-devel, davidedmundson, dfaure, broulik, graesslin, mck182, 
#frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol


D5012: Delay notifications until desktop session has loaded

2017-03-19 Thread Valerio Pilo
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:5bf889f7b94b: Add program to wait for D-Bus Notifications 
services to register. (authored by vpilo).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D5012?vs=12378=12609#toc

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5012?vs=12378=12609

REVISION DETAIL
  https://phabricator.kde.org/D5012

AFFECTED FILES
  startkde/CMakeLists.txt
  startkde/waitforname/CMakeLists.txt
  startkde/waitforname/main.cpp
  startkde/waitforname/org.freedesktop.Notifications.service.in
  startkde/waitforname/waiter.cpp
  startkde/waitforname/waiter.h

To: vpilo, #plasma, #plasma_workspaces, graesslin, dfaure, davidedmundson, 
broulik, mck182
Cc: graesslin, dfaure, davidedmundson, broulik, mck182, plasma-devel, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol


Re: Review Request 129838: fix no-display of CPU bars per core (and fix some warnings)

2017-03-19 Thread Martin Koller


> On Jan. 16, 2017, 8:40 a.m., Kåre Särs wrote:
> > The current/old version uses "connectSource(source)" to add the CPUs when 
> > they are added in onSourceAdded, but that is not good if the sources are 
> > added before SystemLoadViewer.qml (a problem when adding a second 
> > SytemLoadViewer)
> > 
> > I'm fine with this change, but it seems indeed strange that you first only 
> > connect "system/cores"
> > 
> > What happens if you use connectSource(...) in onNewData in stead of setting 
> > all of connectedSources again?
> 
> Martin Koller wrote:
> Sorry, I don't understand your question.
> I'm first connecting to the system/cores only to know how many cores 
> there are since this information is needed to know (calculate) what is the 
> effective sources array I want in the end.
> Therefore:
> step 1) get #cores
> step 2) get list of all needed sources including those which are indexed 
> by the core-idx
> step 3) connect this final list of sources
> 
> What is wrong with this approach ?

Anything I need to clarify ?
or can this path be applied, please ?


- Martin


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


On Jan. 15, 2017, 2:36 p.m., Martin Koller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129838/
> ---
> 
> (Updated Jan. 15, 2017, 2:36 p.m.)
> 
> 
> Review request for Plasma and Kåre Särs.
> 
> 
> Bugs: 373776
> http://bugs.kde.org/show_bug.cgi?id=373776
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> ---
> 
> See bug #373776
> The CPU bars do not show a value when using separate bars per CPU, and the 
> tooltip never
> shows a value per CPU, since the data sources per CPU are not subscribed.
> 
> AFAICT this could never have worked.
> 
> 
> Diffs
> -
> 
>   applets/systemloadviewer/package/contents/ui/CompactBarMonitor.qml 32d98dd 
>   applets/systemloadviewer/package/contents/ui/SystemLoadViewer.qml 5a0bc06 
> 
> Diff: https://git.reviewboard.kde.org/r/129838/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Martin Koller
> 
>