D5143: Introduce fetch-translations build command

2017-03-26 Thread Aleix Pol Gonzalez 
apol added a comment.


  In https://phabricator.kde.org/D5143#97847, @aacid wrote:
  
  > That creates a dependency for ki18n for frameworks that only use .ts files, 
no?
  
  
  It's not really a dependency. It just won't do much over there. AFAIK.
  
  FWIW, I thought this for KI18n framework initially, maybe there's a way to 
better support this for the ts case, I can give it some thought.

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

2017-03-26 Thread Albert Astals Cid 
aacid added a comment.


  That creates a dependency for ki18n for frameworks that only use .ts files, 
no?

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

2017-03-26 Thread Aleix Pol Gonzalez 
apol added a comment.


  In https://phabricator.kde.org/D5143#97840, @aacid wrote:
  
  > why find_package(KF5I18n) ?
  
  
  Because `ki18n_install(po)`

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5143: Introduce fetch-translations build command

2017-03-26 Thread Albert Astals Cid 
aacid added a comment.


  why find_package(KF5I18n) ?

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, ltoscano, aacid
Cc: sitter

D5167: Move .po and .ts files look-up to build-time

2017-03-26 Thread Albert Astals Cid 
aacid added a comment.


  s/Tried/try

REPOSITORY
  R249 KI18n

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

To: apol, #frameworks, sitter, ltoscano, ilic
Cc: aacid

D5167: Move .po and .ts files look-up to build-time

2017-03-26 Thread Albert Astals Cid 
aacid added a comment.


  Tried it in kde-l10n-sr for example

REPOSITORY
  R249 KI18n

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

To: apol, #frameworks, sitter, ltoscano, ilic
Cc: aacid

D5167: Move .po and .ts files look-up to build-time

2017-03-26 Thread Albert Astals Cid 
aacid added a comment.


  Have you tried this?
  
  That commented out test doesn't seem like a good idea.

REPOSITORY
  R249 KI18n

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

To: apol, #frameworks, sitter, ltoscano, ilic
Cc: aacid

D5173: Fix 'Installation of ksendbugmail.exe conflicts with related KDE4 package'.

2017-03-26 Thread Ralf Habacker 
habacker added inline comments.

INLINE COMMENTS

> kbugreport.cpp:527
>  if (command.isEmpty()) {
> -command = QFile::decodeName(CMAKE_INSTALL_PREFIX "/" 
> KF5_LIBEXEC_INSTALL_DIR "/ksendbugmail");
> +command = QFile::decodeName(CMAKE_INSTALL_PREFIX "/" 
> KF5_LIBEXEC_INSTALL_DIR "/ksendbugmail5");
>  }

BTW: This will not work on Windows because the absolute build time install path 
is used which differs in mostly cases from the runtime install path.

REPOSITORY
  R263 KXmlGui

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

To: habacker
Cc: dfaure, aacid, #frameworks

D5173: Fix 'Installation of ksendbugmail.exe conflicts with related KDE4 package'.

2017-03-26 Thread Ralf Habacker 
habacker added a comment.


  I did not took a look into every location where an executable is installed 
into KDE_INSTALL_LIBEXECDIR_KF5, but I guess on Windows any application which 
may be started *only* by a KF5 provided api may be located in the libexec dir ; 
all other needs to stay into /bin (or  in case 
there is no bin dir in the installation I saw in some null soft based 
installations like digikam)
  
  If you really want to have executables in libexec dir on Windows too a patch 
is required to change KDE_INSTALL_LIBEXECDIR_KF5  to xxx/libexec and to 
introduce a different cmake variable for installing all other executables, 
which points to the "bin"-dir on Windows and the same path as 
KDE_INSTALL_LIBEXECDIR_KF5 on other platforms. Then every occurrance of 
KDE_INSTALL_LIBEXECDIR_KF5 in kf5 libraries needs to be checked and replaced by 
this variable
  
  Adding '5' postfix to 4 executables (see 
https://bugs.kde.org/showdependencytree.cgi?id=373928_resolved=1) would be 
a more simple solution from my opinion.

REPOSITORY
  R263 KXmlGui

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

To: habacker
Cc: dfaure, aacid, #frameworks

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
This revision was automatically updated to reflect the committed changes.
Closed by commit R306:c86a0f9f7de9: API dox: fix missing note to call 
setXMLFile with KParts::MainWindow (authored by kossebau).

REPOSITORY
  R306 KParts

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5181?vs=12822=12833

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

AFFECTED FILES
  src/mainwindow.h

To: kossebau, elvisangelaccio, #frameworks, dfaure

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
kossebau added a comment.


  > Also note that lxr finds all calls to setXMLFile, including those in parts 
and plugins, while here we're only talking about kparts mainwindows.
  
  Eh, I had made the extra effort to first do string search for 
"KParts::MainWindow", then manually grep in linked sources for "setXMLFile" and 
"setupGUI" ;) (though just did samples)
  
  Someone should invest into getting something like http://code.woboq.org/ up 
at least next to lxr.kde.org for the whole of KDE repos (santa claus, do you 
listen?).

REPOSITORY
  R306 KParts

BRANCH
  readdsetXMLFileNoteInDox

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

To: kossebau, elvisangelaccio, #frameworks, dfaure

D5185: prevent signals in glib2 be defined by QT

2017-03-26 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Ah, networkmanager-qt itself uses KDEFrameworkCompilerSettings so it gets 
-DQT_NO_SIGNALS_SLOTS_KEYWORDS, i.e. signals isn't defined.
  But indeed this is an installed public header, so the fix is necessary for 
apps that don't use these settings.

REPOSITORY
  R282 NetworkManagerQt

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

To: RobberPhex, #frameworks, dfaure
Cc: dfaure

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  I agree.
  The person who wrote this API doc was aiming for something slightly simpler 
(one less method to call), but my original design was for everyone to call 
setXMLFile, and it would make things a bit weird at the kxmlguiwindow level.
  
  (I just don't agree with the #ifdef argument because one just call setXMLFile 
in all cases, the discussion is more about making things convenient for future 
new code)
  
  Also note that lxr finds all calls to setXMLFile, including those in parts 
and plugins, while here we're only talking about kparts mainwindows. But yeah 
that was the initial design: calling setXMLFile everywhere, in mainwindows, 
parts and plugins. Then some people tried to make the API more compact in 
KXMLGuiWindow, but these KParts API remain basically unchanged.
  
  Anyhow, feel free to ship it.

REPOSITORY
  R306 KParts

BRANCH
  readdsetXMLFileNoteInDox

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

To: kossebau, elvisangelaccio, #frameworks, dfaure

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
kossebau added a comment.


  In https://phabricator.kde.org/D5181#97662, @dfaure wrote:
  
  > Not sure which solution is better.
  
  
  Considered that alternative as well, but then chose to propose the given 
patch for these reasons:
  
  - code supporting KF <5.34 would still need some patching, just having the 
extra `setXMLFile(...)` line without #if-version is less complex code
  - lxr.kde.org hints that lots of existing code uses `setXMLFile(...)`, so 
restoring old API dox hint would match code that exists
  - code not for KParts::MainWindow which does not pass the Create flag would 
need to call createGUI anyway and one would then pass a non-standard ui.rc 
filename in that call
  
  (all assuming  non-standard ui.rc filename of course :) )
  
  The xmlfile argument with the setupGUI() calls feels to be bound to the 
Create flag and not related to the other flags. So if the flag is not passed, 
that argument instintively feels like it should be ignored then.
   Adding support in code and related notion in API dox seems like a 
convenience hack done extra for KParts::MainWindow, where there should be 
rather special code in KParts::MainWindow itself to handle its specifics of the 
UI setup. Even if setupGUI() is a convenience method itself, but that seems too 
much of kitchen-sync. IMHO :) Close call, I agree.

REPOSITORY
  R306 KParts

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

To: kossebau, elvisangelaccio, #frameworks, dfaure

D5185: prevent signals in glib2 be defined by QT

2017-03-26 Thread Robert Lu 
RobberPhex created this revision.
RobberPhex added a project: Frameworks.

REVISION SUMMARY
  Because signals in glib2 be defined by QT, cause Bug 378109 - signals in 
glib2 was defined by QT .
  
  This solved this.

TEST PLAN
  Use nm_conn.cpp ,
  
  before it cannot compiled, after patch, nm_conn.cpp can compiled.

REPOSITORY
  R282 NetworkManagerQt

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

AFFECTED FILES
  src/ipconfig.h

To: RobberPhex, #frameworks

D5173: Fix 'Installation of ksendbugmail.exe conflicts with related KDE4 package'.

2017-03-26 Thread David Faure 
dfaure added a comment.


  If "findable by dbus services" is the only argument for polluting bin with 
libexec binaries on Windows, then it doesn't apply to *everything* in libexec, 
only to a handful of executables.
  
  lib64/libexec/kauth/backlighthelper
  lib64/libexec/kauth/discretegpuhelper
  lib64/libexec/kauth/fontinst
  lib64/libexec/kauth/fontinst_helper
  lib64/libexec/kauth/kalarm_helper
  lib64/libexec/kauth/kcmdatetimehelper
  lib64/libexec/kauth/kcm_kwallet_helper5
  lib64/libexec/kauth/kcmsddm_authhelper
  lib64/libexec/kauth/ksysguardprocesslist_helper
  lib64/libexec/kdeconnectd
  lib64/libexec/kf5/kiod5
  lib64/libexec/kf5/kscreen_backend_launcher
  lib64/libexec/ktp-auth-handler
  lib64/libexec/ktp-filetransfer-handler
  lib64/libexec/ktp-proxy
  lib64/libexec/ktp-text-ui
  
  Everything else can still go into libexec, no?

REPOSITORY
  R263 KXmlGui

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

To: habacker
Cc: dfaure, aacid, #frameworks

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread David Faure 
dfaure added a comment.


  Seems right. The alternative is to make it work.
  Not sure which solution is better.
  
  - kxmlgui/src/kxmlguiwindow.cpp
  
  +++ kxmlgui/src/kxmlguiwindow.cpp
  @@ -203,6 +203,8 @@ void KXmlGuiWindow::setupGUI(const QSize , 
StandardWindowOptions opt
  
if (options & Create) {
createGUI(xmlfile);
  
  +} else if (!xmlfile.isEmpty()) {
  +setXMLFile(xmlfile);
  
}
 
if (d->defaultSize.isValid()) {

REPOSITORY
  R306 KParts

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

To: kossebau, elvisangelaccio, #frameworks, dfaure

D5173: Fix 'Installation of ksendbugmail.exe conflicts with related KDE4 package'.

2017-03-26 Thread Ralf Habacker 
habacker added a comment.


  You saw the related bug report at https://bugs.kde.org/show_bug.cgi?id=374347 
?
  
  "ksendbugmail is installed into KDE_INSTALL_LIBEXECDIR_KF5, which seems to be 
set on Windows to 'bin'. At KDE4 times all executables are installed in 
/bin to be findable by dbus services. This requirement is valid 
for KF5 too so it is required to install the tool with another name 
ksendbugmail5.exe on Windows."

REPOSITORY
  R263 KXmlGui

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

To: habacker
Cc: aacid, #frameworks

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
kossebau edited the summary of this revision.

REPOSITORY
  R306 KParts

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

To: kossebau, dfaure, elvisangelaccio, #frameworks

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
kossebau edited reviewers, added: Frameworks; removed: KDevelop.
kossebau removed a subscriber: Frameworks.

REPOSITORY
  R306 KParts

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

To: kossebau, dfaure, elvisangelaccio, #frameworks

Re: Review Request 128159: Suggest to use setupGUI() instead of setXMLFile()

2017-03-26 Thread Friedrich W. H. Kossebau 

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



Not using "Create" with the setupGUI() call though ran across the initial idea 
of this patch,
as now setXMLFile() is no longer called implicitly when the "Create" flag is 
not passed to
setupGUI() :)

Filed https://phabricator.kde.org/D5181 to fix this up.

- Friedrich W. H. Kossebau


On June 13, 2016, 5:17 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128159/
> ---
> 
> (Updated June 13, 2016, 5:17 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kparts
> 
> 
> Description
> ---
> 
> KXMLGui-based apps should always use `setupGUI()`, which has several 
> advantages over `setXMLFile()` and calls it implicitly. 
> 
> This patch also adds a code snippet about this.
> 
> 
> Diffs
> -
> 
>   src/mainwindow.h b672a35cba92475200ff15620a21150da19582dd 
> 
> Diff: https://git.reviewboard.kde.org/r/128159/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

D5181: API dox: fix missing note to call setXMLFile with KParts::MainWindow

2017-03-26 Thread Friedrich W. H. Kossebau 
kossebau created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  Calling KXmlGuiWindow::setupGUI(flags, xmlfile) without the "Create"
  flag will result in the xmlfile argument being ignored.
  So setXMLFile(xmlfile) needs to be explitly called.
  
  Also
  
  - adds some @c markup for code snippets (will be separate commit)
  - fixes "Default flag" -> "Default flag set"
  - adds KXmlGuiWindow::Create to links for warning, as it has the used info

REPOSITORY
  R306 KParts

BRANCH
  readdsetXMLFileNoteInDox

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

AFFECTED FILES
  src/mainwindow.h

To: kossebau, #kdevelop, dfaure, elvisangelaccio
Cc: #frameworks

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Elvis Angelaccio 


> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1428
> > 
> >
> > I tried to delete a folder (`kioclient5 remove /opt/test/`) but I 
> > didn't get this warning.
> 
> Chinmoy Ranjan Pradhan wrote:
> so was it the warning for file? or auth dialog? or the folder was delted 
> straightaway? In my system I am getting this warning only if the folder is 
> empty.

I only got the auth dialog (the folder was not empty).


- Elvis


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


On March 14, 2017, 2:48 p.m., Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated March 14, 2017, 2:48 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is regarding the GSOC idea 
> https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO.
> 
> This patch intends to demonstrate one possible approach to provide polkit 
> support in kio. Here its only for the delete operation. This is based on the 
> patch in task https://phabricator.kde.org/T5070.
> 
> The approach is as follows;
> 1. Whenever file ioslave gets access denied error it calls the method 
> *execWithRoot* with the action that requires priviledge, the path of items
>upon which action needs to be performed and a warning ID as arguments.
> 2. *execWithRoot* then executes the KAuth::Action *org.kde.kio.file.execute*. 
> 3. This Kauth::Action has its Persistence set too 'session'. This means that 
> after authentication the restrictions are dropped for a while, for
>about 5 minutes. This is similar to the behaviour of sudo command.
> 4. During this time we can perform any action as a privileged user without 
> any authentication. So to prevent any mishap i added a warning box which
>would popup before performing any action(only during this period).
> 5. After the said time interval the root privileges are droped and calling 
> *execWithRoot* should show the usual authentication dialog.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/CMakeLists.txt b9132ce 
>   src/ioslaves/file/file.h 109ea80 
>   src/ioslaves/file/file.cpp eaf6c88 
>   src/ioslaves/file/file_unix.cpp 82eb11a 
>   src/ioslaves/file/kauth/CMakeLists.txt PRE-CREATION 
>   src/ioslaves/file/kauth/file.actions PRE-CREATION 
>   src/ioslaves/file/kauth/helper.h PRE-CREATION 
>   src/ioslaves/file/kauth/helper.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129983/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> warning dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Elvis Angelaccio 


> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > I don't see the advantage of using a single kauth action. This way you 
> > are generating only one "generic" polkit action that you can either enable 
> > or disable. Instead if we used one polkit action per operation (del, rmdir, 
> > etc.) we could allow a more fine-grained control. For example, a sysadmin 
> > could decide that the users can create files in write protected locations, 
> > but they cannot delete existing files.
> 
> Chinmoy Ranjan Pradhan wrote:
> Some operation may use more than one polkit action like Delete. If we use 
> one polkit action per operation then we will have to provide credentials for 
> every polkit action that operation might use. Though we can add necessary 
> code for this but it will only complicate the matter.
> 
> > we could allow a more fine-grained control
> 
> Does this involve editing the policy file directly or just writing a 
> config file? In later case we can provide control over execution of actions 
> within the ioslave. 'execWithRoot' can perform a check prior to execution of 
> the polkit action.
> 
> Elvis Angelaccio wrote:
> > Some operation may use more than one polkit action like Delete. If we 
> use one polkit action per operation then we will have to provide credentials 
> for every polkit action that operation might use.
> 
> But only if the actions are different, right? Otherwise we should be fine 
> within the 5 minutes threshold after the first authentication. Can you show 
> some concrete examples where this issue would happen?
> 
> > Does this involve editing the policy file directly or just writing a 
> config file?
> 
> Have a look at polkit rules. For example, one could write a rule that 
> says "org.kde.kio.file.copy is allowed, org.kde.kio.file.del is not." and it 
> would be very cool if we could actually support this.
> 
> Chinmoy Ranjan Pradhan wrote:
> >But only if the actions are different, right? Otherwise we should be 
> fine within the 5 minutes threshold after the first authentication. Can you 
> show some concrete examples where this issue would happen?
> 
> Delteing a non-empty directory
> In this operation 'del' and 'rmdir' actions are used. In this case we 
> will have to authenticate for both the actions. And after the first 
> authentication, doing a similar operation will show the warning dialog twice.
> 
> Copy operation can also be considered.
> This operation will consist of six actions which are file_open, sendfile, 
> read, write, file_close, chown. Although all the actions will not be used at 
> once usage of atleast two can be expected. Here also we will have to 
> authenticate ouself atleast twice just for copying a single file which is 
> very annoying.
> 
> >it would be very cool if we could actually support this.
> 
> Indeed its very cool and useful,but still, we will have all these 
> problems.
> 
> One possible solution can be using the **annotate** tag. Polkit doc 
> mentions its **used for annotating an action with a key/value pair**. If the 
> _value_ is **org.freedesktop.policykit.imply** then **the subject if 
> authorized for an action with this annotation is also authorized for any 
> action specified by the annotation**.
> So what I did is added this line ``` key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir``` 
> after the defaults tag of _del_ action. After this when I tried deleting a 
> non-empty folder I only got one auth dialog. One more thing I noticed is, 
> with this line in place, when I deleted a single file deleting an empty 
> folder afterwards only showed one warning even though the action was 
> different and was executing for this first time.
> 
> I'm still not sure about this so I might be completely wrong as well. If 
> you have any knowledge of this please let me know if I got everything or 
> anything right. And if what I have mentioned is indeed correct then we will 
> have to add the support for _annotate_ tag to kauth policy generator.

The *imply* annotation sounds very promising, might be exactly what we are 
looking for. Unfortunately I don't know more details about it, at the moment.

Another solution (possibly simpler) could be a middle ground between "only one 
generic action" and "one action per operation". For example, we could have a 
`org.kde.kio.file.delete` action that handles everything about deletion: single 
file, empty folder, non-empty folder, etc.


- Elvis


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


On March 14, 2017, 2:48 p.m., Chinmoy Ranjan

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Chinmoy Ranjan Pradhan 


> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1423
> > 
> >
> > `kioclient5 remove /opt/test/ /opt/test2/` shows this warning, while I 
> > was expecting the one about folders.

Its indeed a logic error. Occurs when the folder contains some files.


> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1428
> > 
> >
> > I tried to delete a folder (`kioclient5 remove /opt/test/`) but I 
> > didn't get this warning.

so was it the warning for file? or auth dialog? or the folder was delted 
straightaway? In my system I am getting this warning only if the folder is 
empty.


- Chinmoy Ranjan


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


On March 14, 2017, 2:48 p.m., Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated March 14, 2017, 2:48 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is regarding the GSOC idea 
> https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO.
> 
> This patch intends to demonstrate one possible approach to provide polkit 
> support in kio. Here its only for the delete operation. This is based on the 
> patch in task https://phabricator.kde.org/T5070.
> 
> The approach is as follows;
> 1. Whenever file ioslave gets access denied error it calls the method 
> *execWithRoot* with the action that requires priviledge, the path of items
>upon which action needs to be performed and a warning ID as arguments.
> 2. *execWithRoot* then executes the KAuth::Action *org.kde.kio.file.execute*. 
> 3. This Kauth::Action has its Persistence set too 'session'. This means that 
> after authentication the restrictions are dropped for a while, for
>about 5 minutes. This is similar to the behaviour of sudo command.
> 4. During this time we can perform any action as a privileged user without 
> any authentication. So to prevent any mishap i added a warning box which
>would popup before performing any action(only during this period).
> 5. After the said time interval the root privileges are droped and calling 
> *execWithRoot* should show the usual authentication dialog.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/CMakeLists.txt b9132ce 
>   src/ioslaves/file/file.h 109ea80 
>   src/ioslaves/file/file.cpp eaf6c88 
>   src/ioslaves/file/file_unix.cpp 82eb11a 
>   src/ioslaves/file/kauth/CMakeLists.txt PRE-CREATION 
>   src/ioslaves/file/kauth/file.actions PRE-CREATION 
>   src/ioslaves/file/kauth/helper.h PRE-CREATION 
>   src/ioslaves/file/kauth/helper.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129983/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> warning dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Chinmoy Ranjan Pradhan 


> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > I don't see the advantage of using a single kauth action. This way you 
> > are generating only one "generic" polkit action that you can either enable 
> > or disable. Instead if we used one polkit action per operation (del, rmdir, 
> > etc.) we could allow a more fine-grained control. For example, a sysadmin 
> > could decide that the users can create files in write protected locations, 
> > but they cannot delete existing files.
> 
> Chinmoy Ranjan Pradhan wrote:
> Some operation may use more than one polkit action like Delete. If we use 
> one polkit action per operation then we will have to provide credentials for 
> every polkit action that operation might use. Though we can add necessary 
> code for this but it will only complicate the matter.
> 
> > we could allow a more fine-grained control
> 
> Does this involve editing the policy file directly or just writing a 
> config file? In later case we can provide control over execution of actions 
> within the ioslave. 'execWithRoot' can perform a check prior to execution of 
> the polkit action.
> 
> Elvis Angelaccio wrote:
> > Some operation may use more than one polkit action like Delete. If we 
> use one polkit action per operation then we will have to provide credentials 
> for every polkit action that operation might use.
> 
> But only if the actions are different, right? Otherwise we should be fine 
> within the 5 minutes threshold after the first authentication. Can you show 
> some concrete examples where this issue would happen?
> 
> > Does this involve editing the policy file directly or just writing a 
> config file?
> 
> Have a look at polkit rules. For example, one could write a rule that 
> says "org.kde.kio.file.copy is allowed, org.kde.kio.file.del is not." and it 
> would be very cool if we could actually support this.

>But only if the actions are different, right? Otherwise we should be fine 
>within the 5 minutes threshold after the first authentication. Can you show 
>some concrete examples where this issue would happen?

Delteing a non-empty directory
In this operation 'del' and 'rmdir' actions are used. In this case we will have 
to authenticate for both the actions. And after the first authentication, doing 
a similar operation will show the warning dialog twice.

Copy operation can also be considered.
This operation will consist of six actions which are file_open, sendfile, read, 
write, file_close, chown. Although all the actions will not be used at once 
usage of atleast two can be expected. Here also we will have to authenticate 
ouself atleast twice just for copying a single file which is very annoying.

>it would be very cool if we could actually support this.

Indeed its very cool and useful,but still, we will have all these problems.

One possible solution can be using the **annotate** tag. Polkit doc mentions 
its **used for annotating an action with a key/value pair**. If the _value_ is 
**org.freedesktop.policykit.imply** then **the subject if authorized for an 
action with this annotation is also authorized for any action specified by the 
annotation**.
So what I did is added this line ```org.kde.kio.file.rmdir``` 
after the defaults tag of _del_ action. After this when I tried deleting a 
non-empty folder I only got one auth dialog. One more thing I noticed is, with 
this line in place, when I deleted a single file deleting an empty folder 
afterwards only showed one warning even though the action was different and was 
executing for this first time.

I'm still not sure about this so I might be completely wrong as well. If you 
have any knowledge of this please let me know if I got everything or anything 
right. And if what I have mentioned is indeed correct then we will have to add 
the support for _annotate_ tag to kauth policy generator.


- Chinmoy Ranjan


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


On March 14, 2017, 2:48 p.m., Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated March 14, 2017, 2:48 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is regarding the GSOC idea 
> https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO.
> 
> This patch intends to demonstrate one possible approach to provide polkit 
> support in kio. Here its

D5178: QtCurve/Qt5 : further KF5 adaptation

2017-03-26 Thread René J . V . Bertin 
rjvbb added a comment.


  KWin5 context menus with an item selected, with my regular light desktop 
theme and the Breeze Dark palette (with and without the patch).
  
  QtCurve is configured to render the menu backdrop with a 100% HSV-based shade 
lightening (the main reason why KWin's context menus become unreadable when the 
client application uses Breeze Dark but not KWin itself).
  
  F3241465: qtcurve-only-winmenu-brdark-unreadable.png 

  
  F3241464: qtcurve-only-winmenu-brdark.png 

  
  F3241463: qtcurve-only-winmenu.png 

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

To: rjvbb, yuyichao
Cc: #frameworks, yuyichao

D5178: QtCurve/Qt5 : further KF5 adaptation

2017-03-26 Thread René J . V . Bertin 
rjvbb added a comment.


  KWin5 context menu when (only) the application uses the Breeze Dark palette, 
with and without the current patch.
  
  F3241430: qtcurve-winmenu-brdark-unreadable.png 

  
  F3241429: qtcurve-winmenu-brdark.png 

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

To: rjvbb, yuyichao
Cc: #frameworks, yuyichao

D5178: QtCurve/Qt5 : further KF5 adaptation

2017-03-26 Thread René J . V . Bertin 
rjvbb added a comment.


  Menu appearance with my regular light desktop palette and when only the 
application (oxygen-demo5) was switched to the Breeze Dark palette.
  
  F3241388: qtcurve-winmenu.png 
  
  F3241387: qtcurve-menu.png 
  
  F3241386: qtcurve-menu-brdark.png 

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

To: rjvbb, yuyichao
Cc: #frameworks, yuyichao

D5178: QtCurve/Qt5 : further KF5 adaptation

2017-03-26 Thread René J . V . Bertin 
rjvbb updated this revision to Diff 12817.
rjvbb added a comment.


  this version attempts to take a few settings into account for rendering the 
items in the special-case KWin context menus.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5178?vs=12799=12817

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

AFFECTED FILES
  qt5/CMakeLists.txt
  qt5/config/CMakeLists.txt
  qt5/style/CMakeLists.txt
  qt5/style/qtcurve.cpp
  qt5/style/qtcurve.h
  qt5/style/qtcurve_api.cpp
  qt5/style/qtcurve_primitive.cpp

To: rjvbb, yuyichao
Cc: #frameworks, yuyichao