D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham marked 4 inline comments as done.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham updated this revision to Diff 21461.
ngraham added a comment.


  Only mark multipage DjVu files as documents, and remove crufty old 
compatibility mimetype that's probably not relevant at all

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8528?vs=21459=21461

BRANCH
  djvu_files_documents_369195

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

AFFECTED FILES
  src/file/basicindexingjob.cpp

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Henrik Fehlauer
rkflx added inline comments.

INLINE COMMENTS

> rkflx wrote in basicindexingjob.cpp:219
> Looking at `/usr/share/mime/image`, I see:
> 
> - `vnd.djvu.xml`: "DjVu image"
> - `vnd.djvu+multipage.xml`: "DjVu document"
> 
> …and that's also what Dolphin shows. Probably best to add `+multipage` here?

Sorry, with "add `+multipage`" I meant to add exactly this to the existing 
line, not to add an additional line. This way, "image" DjVu files are still 
classified as images by baloo, just as in Dolphin.

This might be inconvenient in some cases, but that's how the mimetype standard 
defines it.

> ngraham wrote in basicindexingjob.cpp:220
> https://www.cuminas.jp/docs/djvuplugin/en_us/Content/MIME%20Types.htm said 
> that it was an older one, so I figured I'd add that one for maximum 
> compatibility. I can remove it if you want.

Latest news from djvu.org is from 2013, "older versions //of the DjVu Browser 
Plug-in//" are probably even more prehistoric and won't run in upcoming 
browsers anyway. I don't have this on Tumbleweed, if your Kubuntu does not have 
it I would tend to removal. Also I believe this is just for opening files in 
the browser and not relevant to indexing at all.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham updated this revision to Diff 21459.
ngraham marked an inline comment as done.
ngraham added a comment.


  Actually we don't need the xml extension here

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8528?vs=21458=21459

BRANCH
  djvu_files_documents_369195

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

AFFECTED FILES
  src/file/basicindexingjob.cpp

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham added inline comments.

INLINE COMMENTS

> rkflx wrote in basicindexingjob.cpp:220
> Is `x-djvu` a thing? Not sure, that's why I'm asking.

https://www.cuminas.jp/docs/djvuplugin/en_us/Content/MIME%20Types.htm said that 
it was an older one, so I figured I'd add that one for maximum compatibility. I 
can remove it if you want.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham updated this revision to Diff 21458.
ngraham added a comment.


  Adjusting MIME types to match what's in /usr/share/mime/images

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8528?vs=21455=21458

BRANCH
  djvu_files_documents_369195

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

AFFECTED FILES
  src/file/basicindexingjob.cpp

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8098: Strip down and re-write the tags KIO slave.

2017-10-27 Thread James Smith
smithjd added a comment.


  Deep tag copies (to the filesystem and also in the slave) are known to be 
broken (1 or more folder deep) because of the preview fix in the first revision.

REPOSITORY
  R293 Baloo

BRANCH
  master-nestedTags (branched from master)

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

To: smithjd, #frameworks, vhanda, #dolphin, ngraham
Cc: nicolasfella, ngraham


D8528: Consider DjVu files to be documents

2017-10-27 Thread Henrik Fehlauer
rkflx added a comment.


  General sentiment makes sense, most DjVu documents I've seen were more like 
books. However, see inline comment. (Someone with  actual mimetype or baloo 
knowledge should approve, though.)
  
  > didn't crash baloo with basic usage
  
  If you don't have a DjVu file, that's not "basic usage", but "no usage", 
making it quite pointless to mention the absence of crashes :)
  
  Anyway, see last section here for some example files: 
http://www.djvu.org/resources
  I guess you need to edit your test plan again…

INLINE COMMENTS

> basicindexingjob.cpp:219
>  {"text/markdown", Type::Document},
> +{"image/vnd.djvu", Type::Document},
> +{"image/x-djvu", Type::Document},

Looking at `/usr/share/mime/image`, I see:

- `vnd.djvu.xml`: "DjVu image"
- `vnd.djvu+multipage.xml`: "DjVu document"

…and that's also what Dolphin shows. Probably best to add `+multipage` here?

> basicindexingjob.cpp:220
> +{"image/vnd.djvu", Type::Document},
> +{"image/x-djvu", Type::Document},
>  {"application/x-lyx", Type::Document}

Is `x-djvu` a thing? Not sure, that's why I'm asking.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham added reviewers: vhanda, Frameworks.

REPOSITORY
  R293 Baloo

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

To: ngraham, vhanda, #frameworks
Cc: #frameworks


D8528: Consider DjVu files to be documents

2017-10-27 Thread Nathaniel Graham
ngraham created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  BUG: 369195

TEST PLAN
  Tested in KDE Neon. Compiled and deployed fine; didn't crash baloo with basic 
usage. Unable to test beyond that since I don't have any DjVu files and wasn't 
able to locale any online for direct download or find an online PDF-to-DjVu 
converter that worked

REPOSITORY
  R293 Baloo

BRANCH
  djvu_files_documents_369195

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

AFFECTED FILES
  src/file/basicindexingjob.cpp

To: ngraham
Cc: #frameworks


D7828: createKMessageBox tries to focus a default button when available

2017-10-27 Thread Emirald Mateli
emateli updated this revision to Diff 21453.
emateli added a comment.


  Well, the diff of this patch took a sizeable reduction.
  
  I took a second look at this and here is what happened in the end and what 
caused the inconsistency with Dolphin.
  
  The issue was that the `QDialogButtonBox` argument, which contains the 
buttons needs to have a not null parent argument. If it does, then everything 
works as expected, if it doesn't (which Dolphin does not suppliy) then the 
focus is all messed up and will be given in order to the widgets created that 
can be focused.
  
  From what I dug out the order seems to be:
  
  1. Label (if the `AllowLink` flag is present)
  2. The string list
  3. Checkbox
  4. Left most action button
  
  Moving the call of `setParent` before any of the above mentioned widgets is 
created seems to fix this (I built this system-wide so all the KDE apps running 
can use it and no issues so far, although it's only been a couple of hours)
  
  If this turns out to be okay, I'll edit the summary and title later, since it 
is re-purposed in a way by not involving widget focus change.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7828?vs=20497=21453

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

AFFECTED FILES
  src/kmessagebox.cpp

To: emateli, #frameworks, ngraham, aacid, #vdg
Cc: rkflx, abetts, subdiff, ngraham, aacid, #frameworks


D8527: Display "Downloaded From" by default, if file supports it

2017-10-27 Thread Nathaniel Graham
ngraham edited the test plan for this revision.
ngraham added reviewers: Dolphin, Frameworks.
ngraham added a project: Dolphin.

REPOSITORY
  R824 Baloo Widgets

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

To: ngraham, #dolphin, #frameworks
Cc: spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp


D8437: KWidgetsAddons : more compact password dialog

2017-10-27 Thread René J . V . Bertin
This revision was automatically updated to reflect the committed changes.
Closed by commit R236:9afc2c3768e0: more compact password dialog (authored by 
rjvbb).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8437?vs=21340=21445

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

AFFECTED FILES
  src/kpassworddialog.ui

To: rjvbb, #frameworks, kfunk, ngraham
Cc: cfeck, ngraham


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Flöser
graesslin added a comment.


  Maybe we can drop the force activate for all hosts? Are there any 
implementations left which are on X11? At least Ubuntu is Wayland based.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8098: Strip down and re-write the tags KIO slave.

2017-10-27 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  I have been using this for weeks without any negative effects. I'm giving 
this my blessing, but let's wait for others to weigh in.

REPOSITORY
  R293 Baloo

BRANCH
  master-nestedTags (branched from master)

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

To: smithjd, #frameworks, vhanda, #dolphin, ngraham
Cc: nicolasfella, ngraham


KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 135 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/135/
 Project:
Frameworks kirigami kf5-qt5 XenialQt5.7
 Date of build:
Fri, 27 Oct 2017 15:31:45 +
 Build duration:
1 min 23 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 2 test(s)Failed: TestSuite.qmltests
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(2/2)67%
(6/9)67%
(6/9)64%
(409/640)43%
(169/393)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalssrc80%
(4/5)80%
(4/5)54%
(173/320)34%
(90/263)src.libkirigami50%
(2/4)50%
(2/4)74%
(236/320)61%
(79/130)

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 129 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/129/
 Project:
Frameworks kirigami kf5-qt5 FreeBSDQt5.7
 Date of build:
Fri, 27 Oct 2017 15:31:45 +
 Build duration:
58 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: TestSuite.qmltests

D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread David Edmundson
davidedmundson added a comment.


  Ah, I 1000% agree it /should/ work like that.
  
  My very first comment on here ended with:
  
  > In an ideal world the original SNI spec would have passed a timestamp...
  
  
  
  
  
  So bringing discussion back to this patch.
  
  - We need forceActiveWindow for other hosts/WMs
  
  - From Plasma. I could pass the timestamp to the SNI item via some sideloaded 
extra interface
  
  - This would allow you to ignore forceActiveWindow in kwin without breaking 
this specific thing.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Flöser
graesslin added a comment.


  In https://phabricator.kde.org/D8519#160909, @davidedmundson wrote:
  
  > In https://phabricator.kde.org/D8519#160879, @graesslin wrote:
  >
  > > Actually even on X it is quite simple to get it right. We have the window 
Id, so the host needs to update the xTime in the window. The only question is 
whether Qt reads it back. With an up to date xTime a simple activate without a 
force should work.
  >
  >
  > You're basing off an assumption. That clicking should raise the window. It 
isn't necessarily the case.
  >  There could be no window, (see current keyboard layout item), or new 
windows or something completely different.
  >
  > We could guess some heuristic based on if WindowId is set, dont' call 
activated, but do stuff, but that's in random guessing client-behaviour 
territory.
  
  
  Sorry I didn't express myself correctly. If we have the window id the host 
should update the xTime on the window whenever the associated item is clicked. 
Then the window can raise/activate itself as it has an up to date xTime.
  
  This is for example done in kglobalaccell: the timestamp of the shortcut is 
sent to the application through dbus. The application updates the xTime and 
activates the window. The window manager is happy, the xTime allows it.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8522: keyboard navigation in and out QML kcms

2017-10-27 Thread Marco Martin
mart updated this revision to Diff 21440.
mart added a comment.


  - skip widgets that don't accept focus

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8522?vs=21435=21440

BRANCH
  phab/keynav

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

AFFECTED FILES
  src/kcmoduleqml.cpp
  src/kcmoduleqml_p.h

To: mart, #plasma, #frameworks
Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread David Edmundson
davidedmundson added a comment.


  In https://phabricator.kde.org/D8519#160879, @graesslin wrote:
  
  > Actually even on X it is quite simple to get it right. We have the window 
Id, so the host needs to update the xTime in the window. The only question is 
whether Qt reads it back. With an up to date xTime a simple activate without a 
force should work.
  
  
  You're basing off an assumption. That clicking should raise the window. It 
isn't necessarily the case.
  There could be no window, (see current keyboard layout item), or new windows 
or something completely different.
  
  We could guess some heuristic based on if WindowId is set, dont' call 
activated, but do stuff, but that's in random guessing client-behaviour 
territory.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D7194: Detach before setting the d pointer

2017-10-27 Thread Friedrich W . H . Kossebau
kossebau added a comment.


  In https://phabricator.kde.org/D7194#160789, @leinir wrote:
  
  > This already went into one release, and it would be quite useful to get it 
sorted before the next one rolls around, and the consensus seems to be 
reverting, as leaving kns with this patch in has some fairly unfortunate side 
effects. Could we get it reverted before the next release is done?
  
  
  +1
  
  The issues in Discover should hopefully be fixable by adapting to the 
explicitly-shared nature of the Entry* objects (famous 
non-maintainer/contributor words :) ).
  
  > The point about not reporting value changes is very much a good one, though 
- i think it would make plenty of sense to add that. It would be useful in any 
number of places (including e.g. in QtQuick, which obviously is my little baby, 
if we add it as properties... and it was sort of my mental todo anyway as 
something to do as i went ahead, unless anybody has large objections to me 
doing that).
  
  Most KNS classes have already separate signals to report about changes of 
entries they exposed (driven by KNSCore::Engine::signalEntryChanged), that 
possibly should be relied on or extended. There still might be issues across 
threads with that, but then EntryInternal/Entry objects might not be 
thread-safe anyway?
  
  Looking at the entry-object centric API though I wonder if it might not be 
better to turn for KNS4 to a usage with a central data model stored by the 
entry-maintaining model and consumers using indexes to query/set data when 
needed (seems entries are identifyable by provider + id). That would avoid 
creating lots of Entry* objects with duplicated data on every change of the 
data model and all the extra work by consumers to synchronize their copies of 
the Entry* objects they would maintain.

REPOSITORY
  R304 KNewStuff

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

To: apol, leinir
Cc: sitter, kossebau, whiting, mutlaqja, broulik, #frameworks


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Flöser
graesslin added a comment.


  Actually even on X it is quite simple to get it right. We have the window Id, 
so the host needs to update the xTime in the window. The only question is 
whether Qt reads it back. With an up to date xTime a simple activate without a 
force should work.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Flöser
graesslin added a comment.


  In https://phabricator.kde.org/D8519#160813, @davidedmundson wrote:
  
  > > The removal of the force activation is done since according to kwin 
maintainer it is wrong and must be done by the SysTray itself.
  >
  > But according the the status notifier item maintainer (defacto me), we 
cannot do this. It will break too many things.
  >
  > The SNI spec is used by Unity 7, Gnome, and Plasma. Can we guarauntee their 
trays do this? (in fact even Plasma doesn't currently do this!)
  >
  > I know it's putting you in the middle of an argument, which is unfair on 
you, sorry.
  >
  > forceActiveWindow may be bad, but it works. It's not really focus stealing 
if we explicitly know the user performed the action.  In an ideal world the 
original SNI spec would have passed a timestamp...and blah blah, but yeah.
  
  
  I plan to harden the window activation so that incorrect forceActiveWindow 
doesn't steal focus any more. It is unfortunately a common pattern and KWin in 
that area just sucks. I have a plan to move that forward without breaking stuff 
and addressing Wayland at the same time. Once again thinking in Wayland terms 
helps ;-)

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8522: keyboard navigation in and out QML kcms

2017-10-27 Thread Marco Martin
mart added a reviewer: Frameworks.

REPOSITORY
  R295 KCMUtils

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

To: mart, #plasma, #frameworks
Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D8522: keyboard navigation in and out QML kcms

2017-10-27 Thread Marco Martin
mart created this revision.
mart added a reviewer: Plasma.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  QQuickWidget doesn't support keyboard navigation per se,
  the widget will gain keyboard focus on tab, but lose it again at
  the next tab press. this manages manually tab and backtab in the widget
  with an event filter, using an heuristic to know when the tab focus reached
  the last qtquick component and should go out focusing the next widgets

TEST PLAN
  keyboard navigation with tab/shift+tab works as expected
  both in systemsettings and kcmshell5

REPOSITORY
  R295 KCMUtils

BRANCH
  phab/keynav

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

AFFECTED FILES
  src/kcmoduleqml.cpp
  src/kcmoduleqml_p.h

To: mart, #plasma
Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D8159: Add API for setting server decoration palettes

2017-10-27 Thread Martin Flöser
graesslin added a comment.


  In https://phabricator.kde.org/D8159#160808, @davidedmundson wrote:
  
  > FYI, I'm deliberately delaying after I heard from Jonas how they might put 
it in GTK.
  >  Technically they could have a v1 here, and we could have v2 with the added 
feature set and it would be fine, but I want to see how the discussion plays 
out before modifying anything.
  
  
  If the protocol goes upstream I would be in favor of discarding the change as 
it could mean that also Qt gains support directly. In that case we would need a 
dedicated protocol for color scheme anyway.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

To: davidedmundson, #plasma, graesslin
Cc: graesslin, broulik, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein


D8437: KWidgetsAddons : more compact password dialog

2017-10-27 Thread Nathaniel Graham
ngraham added a comment.


  Go ahead and land this, @rjvbb! Nice work.

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, kfunk, ngraham
Cc: cfeck, ngraham


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Koller
mkoller updated this revision to Diff 21431.
mkoller added a comment.


  In https://phabricator.kde.org/D8519#160813, @davidedmundson wrote:
  
  > > The removal of the force activation is done since according to kwin 
maintainer it is wrong and must be done by the SysTray itself.
  >
  > But according the the status notifier item maintainer (defacto me), we 
cannot do this. It will break too many things.
  
  
  I thought this will be the case ...
  And I still think my previous patch makes it even better than it is now (I 
don't understand why the current code raises the window only when it's already 
shown
  but not otherwise)
  So since you are the maintainer, what is your stance on this ?

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8519?vs=21427=21431

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

AFFECTED FILES
  src/kstatusnotifieritem.cpp

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread David Edmundson
davidedmundson added inline comments.

INLINE COMMENTS

> kstatusnotifieritem.cpp:453
>  d->menu = menu;
> -d->menu->setParent(nullptr);
> +//d->menu->setParent(nullptr);
>  }

make setContextMenu safe against null pointers, prevents creating a dbus menu 
exporter when we don't care for it, prevents re-doing work when calling 
setLegacySystemTrayEnabled when the object is already in the desired state and 
prevents creating multiple KDBusMenuExporter objects if setContextMenu is 
called with the same menu more than once over the lifetime of the menu (which 
can actually happen with this patch).
  
  svn path=/trunk/KDE/kdelibs/; revision=1120745

--

(yes, it really was back in SVN)

We take ownership of the menu in this method, and it's trying to prevent 
accidental double deletions.

It's a bit quirky. If this was new code I wouldn't do it, but again we can't 
risk breaking existing stuff.

It's weird that it changes the window type...that sounds like a bug; I'll 
happily approve setting it back.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread David Edmundson
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  > The removal of the force activation is done since according to kwin 
maintainer it is wrong and must be done by the SysTray itself.
  
  But according the the status notifier item maintainer (defacto me), we cannot 
do this. It will break too many things.
  
  The SNI spec is used by Unity 7, Gnome, and Plasma. Can we guarauntee their 
trays do this? (in fact even Plasma doesn't currently do this!)
  
  I know it's putting you in the middle of an argument, which is unfair on you, 
sorry.
  
  forceActiveWindow may be bad, but it works. It's not really focus stealing if 
we explicitly know the user performed the action.  In an ideal world the 
original SNI spec would have passed a timestamp...and blah blah, but yeah.
  
  For Wayland this won't work and will do nothing, which is fine. We do need to 
fix that properly in a different manner, but lets not break X in the meantime.

REPOSITORY
  R289 KNotifications

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

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8159: Add API for setting server decoration palettes

2017-10-27 Thread David Edmundson
davidedmundson added a comment.


  FYI, I'm deliberately delaying after I heard from Jonas how they might put it 
in GTK.
  Technically they could have a v1 here, and we could have v2 with the added 
feature set and it would be fine, but I want to see how the discussion plays 
out before modifying anything.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

To: davidedmundson, #plasma, graesslin
Cc: graesslin, broulik, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein


KDE CI: Frameworks kservice kf5-qt5 XenialQt5.7 - Build # 29 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kservice%20kf5-qt5%20XenialQt5.7/29/
 Project:
Frameworks kservice kf5-qt5 XenialQt5.7
 Date of build:
Fri, 27 Oct 2017 11:23:38 +
 Build duration:
2 min 16 sec and counting
   JUnit Tests
  Name: (root) Failed: 5 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: TestSuite.kmimeassociationstestFailed: TestSuite.kplugininfotestFailed: TestSuite.kservicetestFailed: TestSuite.ksycoca_xdgdirstestFailed: TestSuite.ksycocathreadtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report86%
(6/7)87%
(73/84)87%
(73/84)57%
(4459/7840)36%
(2195/6063)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(13/13)100%
(13/13)82%
(1127/1382)41%
(663/1617)src.kbuildsycoca100%
(1/1)100%
(1/1)91%
(61/67)65%
(13/20)src.kdeinit0%
(0/2)0%
(0/2)0%
(0/326)0%
(0/262)src.plugin67%
(2/3)67%
(2/3)47%
(47/100)38%
(36/96)src.services90%
(28/31)90%
(28/31)52%
(1575/3046)33%
(625/1904)src.sycoca84%
(26/31)84%
(26/31)55%
(1541/2799)39%
(824/2114)tests.pluginlocator100%
(3/3)100%
(3/3)90%
(108/120)68%
(34/50)

D8519: do not make the context menu a Window; do not force raise a window

2017-10-27 Thread Martin Koller
mkoller created this revision.
mkoller added reviewers: davidedmundson, graesslin.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  The setParent() call in this patch was changing the window flags from Popup 
to Window, which leads to show the popup with a window frame when activated via 
a SysTray DBus call.
  I don't know why setParent() is done here in the first place (could not find 
the change in git history).
  If setParent() should still be done, at least the window flag must be set 
back to Popup.
  
  The removal of the force activation is done since according to kwin 
maintainer it is wrong and must be done by the SysTray itself.

REPOSITORY
  R289 KNotifications

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

AFFECTED FILES
  src/kstatusnotifieritem.cpp

To: mkoller, davidedmundson, graesslin
Cc: #frameworks


D8188: Remove PreferCache from network requests

2017-10-27 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Right, yes, i think that was a bit of left over from working on it all. Go 
for it.

REPOSITORY
  R304 KNewStuff

BRANCH
  master

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

To: aacid, leinir
Cc: #frameworks


KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.7 - Build # 126 - Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.7/126/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.7
 Date of build:
Fri, 27 Oct 2017 10:48:17 +
 Build duration:
21 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 53 test(s)Failed: TestSuite.kiocore-ktcpsockettestFailed: TestSuite.kiowidgets-kdirmodeltest

KDE CI: Frameworks kio kf5-qt5 XenialQt5.7 - Build # 125 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20XenialQt5.7/125/
 Project:
Frameworks kio kf5-qt5 XenialQt5.7
 Date of build:
Fri, 27 Oct 2017 10:48:17 +
 Build duration:
17 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 54 test(s)Failed: TestSuite.kiowidgets-kurifiltersearchprovideractionstestFailed: TestSuite.kiowidgets-kurifiltertest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(22/22)80%
(278/346)80%
(278/346)57%
(29955/52254)42%
(16564/39297)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(67/67)100%
(67/67)96%
(8078/8399)51%
(4505/8824)autotests.http100%
(9/9)100%
(9/9)100%
(543/544)60%
(200/336)autotests.kcookiejar100%
(1/1)100%
(1/1)90%
(179/198)67%
(60/90)src.core85%
(99/117)85%
(99/117)57%
(8149/14198)49%
(4551/9279)src.core.kssl100%
(1/1)100%
(1/1)38%
(35/93)50%
(3/6)src.filewidgets73%
(27/37)73%
(27/37)46%
(3488/7600)30%
(1299/4401)src.gui100%
(2/2)100%
(2/2)95%
(104/110)75%
(54/72)src.ioslaves.file100%
(2/2)100%
(2/2)51%
(435/849)42%
(317/749)src.ioslaves.http100%
(8/8)100%
(8/8)47%
(1762/3783)38%
(1317/3462)src.ioslaves.http.kcookiejar100%
(2/2)100%
(2/2)80%
(622/782)72%
(608/839)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(70/258)8%
(16/200)src.ioslaves.trash80%
(8/10)80%
(8/10)60%
(704/1173)47%
(404/851)src.ioslaves.trash.tests100%
(2/2)100%
(2/2)90%
(707/785)48%
(461/970)src.kioslave100%
(1/1)100%
(1/1)52%
(14/27)50%
(5/10)src.kntlm100%
(2/2)100%
(2/2)97%
(373/385)80%
(111/138)src.kpasswdserver100%
(2/2)100%
(2/2)64%
(382/594)49%
(285/580)src.kpasswdserver.autotests100%
(1/1)100%
(1/1)99%
(283/286)57%

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

2017-10-27 Thread Simone Gaiarin
simgunz added a comment.


  > Can we not just remove the drop down menu and always show the checkbox for 
terminal?
  
  Actually I've just changed that behaviour few months ago to follow the 
"simple by default, powerful when needed" philosophy. See this review and the 
related bug for the details:
  https://git.reviewboard.kde.org/r/130180/
  https://bugs.kde.org/show_bug.cgi?id=359233

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

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

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks


D7194: Detach before setting the d pointer

2017-10-27 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  This already went into one release, and it would be quite useful to get it 
sorted before the next one rolls around, and the consensus seems to be 
reverting, as leaving kns with this patch in has some fairly unfortunate side 
effects. Could we get it reverted before the next release is done?
  
  The point about not reporting value changes is very much a good one, though - 
i think it would make plenty of sense to add that. It would be useful in any 
number of places (including e.g. in QtQuick, which obviously is my little baby, 
if we add it as properties... and it was sort of my mental todo anyway as 
something to do as i went ahead, unless anybody has large objections to me 
doing that).

REPOSITORY
  R304 KNewStuff

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

To: apol, leinir
Cc: sitter, kossebau, whiting, mutlaqja, broulik, #frameworks


KDE CI: Frameworks kio kf5-qt5 XenialQt5.7 - Build # 124 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20XenialQt5.7/124/
 Project:
Frameworks kio kf5-qt5 XenialQt5.7
 Date of build:
Fri, 27 Oct 2017 10:03:21 +
 Build duration:
17 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 52 test(s), Skipped: 0 test(s), Total: 54 test(s)Failed: TestSuite.kiowidgets-kurifiltersearchprovideractionstestFailed: TestSuite.kiowidgets-kurifiltertest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(22/22)80%
(278/346)80%
(278/346)57%
(29960/52252)42%
(16560/39297)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(67/67)100%
(67/67)96%
(8078/8399)51%
(4501/8824)autotests.http100%
(9/9)100%
(9/9)100%
(543/544)60%
(200/336)autotests.kcookiejar100%
(1/1)100%
(1/1)90%
(179/198)67%
(60/90)src.core85%
(99/117)85%
(99/117)57%
(8157/14198)49%
(4555/9279)src.core.kssl100%
(1/1)100%
(1/1)38%
(35/93)50%
(3/6)src.filewidgets73%
(27/37)73%
(27/37)46%
(3490/7600)30%
(1300/4401)src.gui100%
(2/2)100%
(2/2)95%
(104/110)75%
(54/72)src.ioslaves.file100%
(2/2)100%
(2/2)51%
(435/849)42%
(317/749)src.ioslaves.http100%
(8/8)100%
(8/8)47%
(1762/3781)38%
(1317/3462)src.ioslaves.http.kcookiejar100%
(2/2)100%
(2/2)80%
(622/782)72%
(608/839)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(70/258)8%
(16/200)src.ioslaves.trash80%
(8/10)80%
(8/10)60%
(704/1173)47%
(404/851)src.ioslaves.trash.tests100%
(2/2)100%
(2/2)90%
(707/785)48%
(461/970)src.kioslave100%
(1/1)100%
(1/1)52%
(14/27)50%
(5/10)src.kntlm100%
(2/2)100%
(2/2)97%
(373/385)80%
(111/138)src.kpasswdserver100%
(2/2)100%
(2/2)63%
(377/594)48%
(280/580)src.kpasswdserver.autotests100%
(1/1)100%
(1/1)99%
(283/286)57%

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.7 - Build # 124 - Fixed!

2017-10-27 Thread CI System
BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.7/124/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.7
 Date of build:
Fri, 27 Oct 2017 09:48:41 +
 Build duration:
18 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 53 test(s)

KDE CI: Frameworks kio kf5-qt5 XenialQt5.7 - Build # 123 - Still Unstable!

2017-10-27 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20XenialQt5.7/123/
 Project:
Frameworks kio kf5-qt5 XenialQt5.7
 Date of build:
Fri, 27 Oct 2017 09:48:41 +
 Build duration:
14 min and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 54 test(s)Failed: TestSuite.kiowidgets-accessmanagertestFailed: TestSuite.kiowidgets-kurifiltersearchprovideractionstestFailed: TestSuite.kiowidgets-kurifiltertest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report100%
(22/22)80%
(278/346)80%
(278/346)57%
(29982/52255)42%
(16596/39299)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(67/67)100%
(67/67)96%
(8078/8399)51%
(4510/8824)autotests.http100%
(9/9)100%
(9/9)100%
(543/544)60%
(200/336)autotests.kcookiejar100%
(1/1)100%
(1/1)90%
(179/198)67%
(60/90)src.core85%
(99/117)85%
(99/117)58%
(8165/14198)49%
(4572/9279)src.core.kssl100%
(1/1)100%
(1/1)38%
(35/93)50%
(3/6)src.filewidgets73%
(27/37)73%
(27/37)46%
(3490/7603)30%
(1299/4403)src.gui100%
(2/2)100%
(2/2)95%
(104/110)75%
(54/72)src.ioslaves.file100%
(2/2)100%
(2/2)52%
(441/849)43%
(322/749)src.ioslaves.http100%
(8/8)100%
(8/8)47%
(1770/3781)38%
(1326/3462)src.ioslaves.http.kcookiejar100%
(2/2)100%
(2/2)80%
(622/782)72%
(608/839)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(70/258)8%
(16/200)src.ioslaves.trash80%
(8/10)80%
(8/10)60%
(704/1173)47%
(404/851)src.ioslaves.trash.tests100%
(2/2)100%
(2/2)90%
(707/785)48%
(461/970)src.kioslave100%
(1/1)100%
(1/1)52%
(14/27)50%
(5/10)src.kntlm100%
(2/2)100%
(2/2)97%
(373/385)80%
(111/138)src.kpasswdserver100%
(2/2)100%
(2/2)63%
(377/594)48%
(280/580)src.kpasswdserver.autotests100%
(1/1)100%
(1/1)99%
(283/286)56%

D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

2017-10-27 Thread Roman Gilg
subdiff added a comment.


  In https://phabricator.kde.org/D8056#159636, @fabianr wrote:
  
  > Not really on topic, but could you change the label for the "Advanced 
options" to something more meaningful, eg "Terminal options" ? The HIG 
recommends not to use the label "Advanced options", but " Use a descriptive 
label so that it reflects the functionality. "
  
  
  Can we not just remove the drop down menu and always show the checkbox for 
terminal?
  
  @simgunz Please do the change to this though in a separate patch. And 
regarding your patch here please give @dfaure a few days to review it as well. 
Also @abetts should give his ok from VDG.
  
  I personally like the change. +1

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

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

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: subdiff, fabianr, abetts, ngraham, alexeymin, #frameworks


D8056: Improve usability of "Open With" dialog by adding option to filter the application tree

2017-10-27 Thread Simone Gaiarin
simgunz added a comment.


  @ngraham Great! I don't have commit access.
  
  @fabianr I agree on changing that label to "Terminal options". I'll send a 
new revision request for that.

REPOSITORY
  R241 KIO

BRANCH
  openwithdialog-filter-app-tree

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

To: simgunz, dfaure, #frameworks, #vdg, ngraham
Cc: fabianr, abetts, ngraham, alexeymin, #frameworks


D8437: KWidgetsAddons : more compact password dialog

2017-10-27 Thread Kevin Funk
kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  Looks like it :)

REPOSITORY
  R236 KWidgetsAddons

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

To: rjvbb, #frameworks, kfunk, ngraham
Cc: cfeck, ngraham