D4995: Fix DB inconsistency due to some docterms appearing with uppercase symbols

2017-03-09 Thread Igor Poboiko
poboiko added reviewers: pinakahuja, vhanda.

REPOSITORY
  R293 Baloo

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

To: poboiko, pinakahuja, vhanda
Cc: #frameworks


D4995: Fix DB inconsistency due to some docterms appearing with uppercase symbols

2017-03-09 Thread Igor Poboiko
poboiko created this revision.
poboiko added a project: Frameworks.

REVISION SUMMARY
  I've noted that on some PDF files, "balooshow -x file.pdf" segfaulted. 
Backtrace showed that it crashed due to having single "X" term (see line 201) 
. 
Moreover, it actually had a bunch of terms containing uppercase symbols (which 
should never occur, all the search terms are lowercase and uppercase is 
reserved for metadata).
  Further investigation showed that pdf file (after extraction) contained 
exotic unicode symbols (ex.: "퐻푒푑푔푒"). After casting toLower(), that string 
remained the same; and after normalization it became "Hedge", and with that 
uppercase symbols it went right to DB.

TEST PLAN
  I've tested it on affected file; "balooshow -x" no longer crashes and no 
longer contains uppercase terms.
  
  Probably one can add additional check for "balooctl checkDb" command for that 
problematic case.
  I can prepare a separate patch, if necessary.

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  src/engine/termgenerator.cpp

To: poboiko
Cc: #frameworks


Re: Review Request 129985: [kio-extras] Thumbs for audio files

2017-03-09 Thread Anthony Fieroni

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



Any other downsides?

- Anthony Fieroni


On Март 6, 2017, 7:44 преди обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129985/
> ---
> 
> (Updated Март 6, 2017, 7:44 преди обяд)
> 
> 
> Review request for KDE Frameworks, David Faure and Elvis Angelaccio.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> ---
> 
> ^^
> 
> 
> Diffs
> -
> 
>   cmake/FindTaglib.cmake PRE-CREATION 
>   thumbnail/CMakeLists.txt 7a1ee45a 
>   thumbnail/audiocreator.h PRE-CREATION 
>   thumbnail/audiocreator.cpp PRE-CREATION 
>   thumbnail/audiothumbnail.desktop PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129985/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>



D4799: Delay notifications until desktop session has loaded

2017-03-09 Thread Martin Klapetek
mck182 added a comment.


  Looks like this could work. The `KDE_FULL_SESSION` check is
  really old, I'm not sure if this has any implications.
  
  As David noted, the waiting thing should go to Plasma workspace,
  that's where it belongs. Also, it would be preferred if the implementation
  was properly split into main.cpp and your class .h and .cpp

INLINE COMMENTS

> main.cpp:39
> +Q_OBJECT
> +private:
> +constexpr static const int dbusTimeoutSec = 60;

`private` in our code always goes to the bottom

REPOSITORY
  R289 KNotifications

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

To: vpilo
Cc: davidedmundson, dfaure, broulik, graesslin, mck182, #frameworks


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

2017-03-09 Thread Chinmoy Ranjan Pradhan

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

(Updated March 9, 2017, 3:57 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 (updated)
-

  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



D4972: Start drag with press and hold on touch events

2017-03-09 Thread Marco Martin
mart added a comment.


  the discussion seems to me a bit unrelated with the review in itself.
  for me the patch is ok, +1

REPOSITORY
  R296 KDeclarative

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

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


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

2017-03-09 Thread Chinmoy Ranjan Pradhan


> On March 6, 2017, 11:38 p.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/file/file.cpp, line 1378
> > 
> >
> > I'm not convinced, what's the point of requestroot? Why don't you let 
> > it call the action right away?
> 
> Chinmoy Ranjan Pradhan wrote:
> For a single file calling action right away might work but doing this for 
> multiple files will show the authentication dialog everytime before file is 
> deleted. This is something we don't want. 
> One way to solve this is to store all the paths and delete them in helper 
> but this approach might  cause problem while porting FileProtocol::copy.
> 
> Elvis Angelaccio wrote:
> > doing this for multiple files will show the authentication dialog 
> everytime before file is deleted.
> 
> This should not happen, sounds like a bug in KAuth? With 
> `Persistence=session` the authorization is supposed to last until the user 
> logs out, according to 
> https://api.kde.org/frameworks/kauth/html/namespaceKAuth.html

It happened because I ommited the Persistence=session statement and this 
behaviour is not a bug. 
My previous comment was somewhat misleading, really sorry for that.


- Chinmoy Ranjan


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


On March 9, 2017, 2:57 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 9, 2017, 2:57 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;
> The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
> unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
> gets two actions, /*org.kde.kio.file.requestroot*/ and 
> /*org.kde.kio.file.execute*/. 
> When an action encounters access denied error the method "execWithRoot" is 
> called with the action you want to perform and the path of objects upon which 
> you want to perform the action as arguments. This method then calls 
> "getRootPermission" for authorisation purpose. Upon successfull authorisation 
> this will then go on performing the desired action as privileged user. Once 
> the job is finished "unsetRoot" is called.
> For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
> action has its "Policy" set to "auth_admin" so as to prompt for password 
> every time its called. And the action "org.kde.kio.file.execute" has its 
> "Policy" set to "yes" so that it can carry out the desired action as a 
> priviledged user without asking for authentication. 
> 
> As for the deletion of files and directories are concerned, the 
> authentication dialog will pop up only once i.e, for the first file/directory 
> that needs requires a priviledged user to delete them. If there are more 
> files which only priviledge users can delete then they will be deleted 
> straightaway without asking for authentication. This is decided by the truth 
> of variable "isRoot". Once the delete job is finished "isRoot" is set to 
> false. In short once the job has started and authentication's been done, the 
> root access will persist and once the job is finished the root access will 
> reset.
> 
> 
> 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-09 Thread Chinmoy Ranjan Pradhan

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

(Updated March 9, 2017, 2:57 p.m.)


Review request for KDE Frameworks, David Faure and Elvis Angelaccio.


Changes
---

Addressed Aleix's issue.
Removed 'requestroot' method which was pointless.
Added method to display warning dialog if root access persists.


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;
The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
gets two actions, /*org.kde.kio.file.requestroot*/ and 
/*org.kde.kio.file.execute*/. 
When an action encounters access denied error the method "execWithRoot" is 
called with the action you want to perform and the path of objects upon which 
you want to perform the action as arguments. This method then calls 
"getRootPermission" for authorisation purpose. Upon successfull authorisation 
this will then go on performing the desired action as privileged user. Once the 
job is finished "unsetRoot" is called.
For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
action has its "Policy" set to "auth_admin" so as to prompt for password every 
time its called. And the action "org.kde.kio.file.execute" has its "Policy" set 
to "yes" so that it can carry out the desired action as a priviledged user 
without asking for authentication. 

As for the deletion of files and directories are concerned, the authentication 
dialog will pop up only once i.e, for the first file/directory that needs 
requires a priviledged user to delete them. If there are more files which only 
priviledge users can delete then they will be deleted straightaway without 
asking for authentication. This is decided by the truth of variable "isRoot". 
Once the delete job is finished "isRoot" is set to false. In short once the job 
has started and authentication's been done, the root access will persist and 
once the job is finished the root access will reset.


Diffs (updated)
-

  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 (updated)


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



D4799: Delay notifications until desktop session has loaded

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


  Cool, but we should put the blocking code in plasma, not here. Otherwise we'd 
interefere with other platforms.
  Maybe in plasma-workspace/startkde

REPOSITORY
  R289 KNotifications

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

To: vpilo
Cc: davidedmundson, dfaure, broulik, graesslin, mck182, #frameworks


Communication with Phabricator upstream

2017-03-09 Thread Ben Cooksley
Hi everyone,

Just repeating my last email on this subject as it seems some folks
have missed the previous memo.

All issues with Phabricator should be logged at
https://phabricator.kde.org/tag/phabricator/ - not with upstream.

This is being done to avoid duplicate tasks, and to allow us to
communicate on a whole of project basis the things we consider
important. This helps simplify things for upstream.

Thanks,
Ben Cooksley
KDE Sysadmin