Re: Review Request 130013: Make PlainTextExtractor match "text/plain" mimetype

[Anthony Fieroni]

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


Ship it!




Ship It!

- Anthony Fieroni


On Март 14, 2017, 11:16 след обяд, Igor Poboiko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130013/
> ---
> 
> (Updated Март 14, 2017, 11:16 след обяд)
> 
> 
> Review request for KDE Frameworks and Anthony Fieroni.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> After commit 7c7e985a4678fef5f5d0dd8faa9b9cb42e3844b4 (see 
> https://git.reviewboard.kde.org/r/129720/), PlainTextExtractor no longer 
> matches ANY of the text/ mimetypes.
> This broke completely Baloo indexing e.g. simple plain text files.
> Introduced check however allows to provide "text/plain" as supported mimetype 
> for the extractor and hope that everything containing plain text will be 
> inherited from it.
> 
> 
> Diffs
> -
> 
>   src/extractors/plaintextextractor.cpp 26e1247 
> 
> Diff: https://git.reviewboard.kde.org/r/130013/diff/
> 
> 
> Testing
> ---
> 
> KFileMetaData compiles.
> Baloo indexes plain text files.
> Everybody is happy.
> 
> 
> Thanks,
> 
> Igor Poboiko
> 
>

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

[Elvis Angelaccio]

> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.
> 
> Chinmoy Ranjan Pradhan wrote:
> In my system when i use Shift+del there is no warning dialog (few months 
> back dolphin used to show the dialog but strangly it doesn't shows now) so 
> this case didn't occured to me. 
> 
> If I am not wrong jobuidelegate's warning is shown before the job 
> executes and kauth's authentication dialog is shown only after job executes. 
> But its just the opposite we want. So if there's a way for slave to notify 
> the job of starting of a priviledged action then there should be no problem 
> in making jobuidelegate smarter. 
> (But the thing is at this very moment i can't think of or find any 
> possible way)

You probably checked the "don't show again" checkbox ;) The jobuidelegate's 
dialog is shown by default, so we must assume the user will see it => we need a 
way to prevent showing the dialog twice. You are right that jobuidelegate is 
run before kio_file is involved. I was thinking of checking the permissions 
from jobuidelegate: can you check if QFileInfo would do the trick? (e.g. 
QFileInfo::isWritable()?)


- Elvis


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


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
> 
>

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb updated this revision to Diff 12480.
rjvbb added a comment.


  - don't use QElapsedTimer::elapsed() when the timer isn't valid. Changes 
nothing to the behaviour but may make the code a bit less obfuscated.
  - edited a few comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5037?vs=12466&id=12480

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

AFFECTED FILES
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

To: rjvbb, #ktexteditor
Cc: luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, 
#frameworks, head7, cullmann, kfunk, sars

Review Request 130013: Make PlainTextExtractor match "text/plain" mimetype

[Igor Poboiko]

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

Review request for KDE Frameworks and Anthony Fieroni.


Repository: kfilemetadata


Description
---

After commit 7c7e985a4678fef5f5d0dd8faa9b9cb42e3844b4 (see 
https://git.reviewboard.kde.org/r/129720/), PlainTextExtractor no longer 
matches ANY of the text/ mimetypes.
This broke completely Baloo indexing e.g. simple plain text files.
Introduced check however allows to provide "text/plain" as supported mimetype 
for the extractor and hope that everything containing plain text will be 
inherited from it.


Diffs
-

  src/extractors/plaintextextractor.cpp 26e1247 

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


Testing
---

KFileMetaData compiles.
Baloo indexes plain text files.
Everybody is happy.


Thanks,

Igor Poboiko

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb added a comment.


  In https://phabricator.kde.org/D5037#95031, @anthonyfieroni wrote:
  
  > Ok, note this situation, hold modifier before first ever wheel event => you 
call m_lastWheelEvent.elapsed on unstarted timer
  
  
  This is true, and things can indeed be improved a bit there.
  
  > and function returns true (m_accidentalModifier == false, 
m_lastWheelEventUnmodified == false)
  
  Am I missing something? For me that is exactly what the function should 
return, and the internal state is correct too. This works because the initial 
values for the 2 boolean state variables are set appropriately. In this case 
`deltaT` may be assigned a random value (because ElapsedSince::elapsed() is 
undefined) but that value won't be used.
  
  I can rewrite the code slightly so that the timer isn't used when not started 
but it won't make a difference for the end result, it will just add a few lines 
and an additional runtime check. Will do that ASAP.
  
  I'd say try the patch and see if you can break its (now of after I upload the 
rewritten version). I just tried generating wheel events with the control 
modifier set from event 1. I even "primed" the text zoom via the keyboard 
shortcuts to prevent initial zooming hickups. I don't notice anything weird: 
the zoom works just as you'd expect.
  
  Let me repeat if this wasn't clear: the purpose of this patch is only to 
prevent accidental zooming because you push the Control key while a scroll is 
still in progress. There's no way we can decide if a Control press is 
accidental when nothing else is going on.
  
  In fact, we don't even know if what I now call an accidental Control press is 
really accidental. It's a reasonable assumption (I think) and it's still easy 
enough to trigger wheel-zooming with this protection in place. But it is 
possible that in a certain number of cases that we now treat as regular scroll 
events the user actually wanted zooming, not scrolling. I just have to hope 
that it is easier for those users to adapt to the behaviour with protection 
than it is for users like me to adapt to not having it :)

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

To: rjvbb, #ktexteditor
Cc: luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, 
#frameworks, head7, cullmann, kfunk, sars

D5037: KTextEditor: prevent accidental zooming

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ok, note this situation, hold modifier before first ever wheel event => you 
call m_lastWheelEvent.elapsed on unstarted timer and function returns true 
(m_accidentalModifier == false, m_lastWheelEventUnmodified == false)

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

To: rjvbb, #ktexteditor
Cc: luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, 
#frameworks, head7, cullmann, kfunk, sars

D5034: Add support for x-gvfs style options in fstab

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  The patch looks good to me (without testing this locally here). I think the 
one QStringRef can be turned by to QStringList, since you finally need the 
QString anyways it seems...

INLINE COMMENTS

> fstabhandling.cpp:152
> +for (const QStringRef &option : options) {
> +globalFstabCache->m_fstabOptionsCache.insert(device, 
> option.toString());
> +}

Well ok, given you need to convert the string to QString finally anyways, you 
can keep the QStringList above (line 148).

REPOSITORY
  R245 Solid

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

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

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb added a subscriber: luebking.

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

To: rjvbb, #ktexteditor
Cc: luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, 
#frameworks, head7, cullmann, kfunk, sars

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb added a comment.


  In https://phabricator.kde.org/D5037#94944, @anthonyfieroni wrote:
  
  > Ok, we discard first wheel event cause elapsed timer isn't started, right?
  
  
  No, it's not discarded; no wheel events are being discarded unless they 
already were being discarded. What my addition does is turn certain Ctrl+Wheel 
events into regular wheel events so that the view continues to scroll when you 
press the modified during a scroll (instead of switching to zooming).
  
  There is no previous event so there's no need to do any checks to see if the 
modifier should be allowed to trigger zooming. So the first wheelevent just 
passes through the filter unmodified and if the modifier is pressed it will 
trigger zooming. Any other approach will probably only make things more 
difficult. And FWIW it's already semantically doubtful to set 
m_lastWheelEventUnmodified to false in the WheelEventFilter ctor (there is only 
a last event after the 1st wheel event has been processed) so putting it in the 
KateViewInternal ctor makes even less sense.
  
  Similarly, the timer must be specific to wheel events, you wouldn't want 
mouse movements to interfere for instance. Or at least I think so, I only have 
trackpads nowadays.
  
  Setting the modifier key in KateViewInternal does make sense but more so when 
it actually becomes possible to change it. Until that's not the case the 
compiler can optimise the current implementation a bit better (as far as that's 
of any importance here).
  
  > I think we can "catch" all events we just make it a bit different
  
  I don't think I understand what you mean here. What we don't catch is 
situations where the events tell the view to continue scrolling in a direction 
where that's no longer possible. My PoC implementation on github catches those 
situations but would be overkill (and probably interfere with zooming when the 
document is at the top or bottom).
  
  As to dropping wheel events: that doesn't happen but even if it did that 
shouldn't be a big deal if only 1 or a few events are dropped. This kind of 
event arrive in bursts from an effect-driven user input that isn't transient. 
IOW, contrary to clicking something, s/he will continue to turn the wheelbutton 
(or swipe up/down) until satisfied with whatever the desired result. I strongly 
doubt anyone one would notice if we dropped the 1st wheel event for instance.

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

To: rjvbb, #ktexteditor
Cc: anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, 
cullmann, kfunk, sars

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

[Chinmoy Ranjan Pradhan]

> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/kauth/helper.cpp, line 14
> > 
> >
> > Why is this using a QueuedConnection? This prevents any sort of error 
> > handling because this method returns too early.
> > 
> > Testing that invokeMethod worked is only half the story. If the 
> > QueuedConnection can be removed, you can use a return argument of type bool 
> > for error handling?
> > Or better, a KIO error code, so we can give the exact error message (I 
> > see that we didn't have "directory not empty" for rmdir, but if this is 
> > extended to other actions then it will be useful to have).
> > 
> > In fact, instead of invokeMethod this could be a set of if(method == 
> > "...")? Seems simpler to me, but I have no strong opinion.

I used invokeMethod for compactness but an if-else construct is indeed simpler


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/kauth/helper.cpp, line 26
> > 
> >
> > This statement does nothing, the method would return anyway immediately 
> > afterwards. Is there missing error handling here? Shouldn't this return a 
> > bool?

Error handling was supposed to be there but somehow I messed up.


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/file.cpp, line 1391
> > 
> >
> > can you explain the meaning of the isRoot member to me? It sounds like 
> > "I am authorized to be root", but here it's set while in "auth required" 
> > state, sounds like we're not root yet?
> > (I know nothing about polkit/kauth but at least this code should make 
> > sense to me ;)

The name isRoot is misleading so I changed it to mPriviledgeOpStarted. It is 
used to show that the priviledged operation has started and will be set to 
false once the job ends. This along with kauthStatus are used to decide whether 
to show warning dialog or not.

The "execute" method in KAuth is similar to sudo command. The priviledge 
persist for about 5 minutes. It is during this time we need to show the 
warning. Now when deleting multiple files we can either delete them using one 
job(deleteRecursive) or multiple jobs. So to determine whether the call to 
execWithRoot was from the same job or not and show the warning accordingly I 
used the aforementioned members.

So my logic here is,
- Prepare the arguments
- If kauth status is Auth Required status(i.e. execWithRoot is called for the 
first time) then set mPriviledgeOpStarted to true.
- If kauth status is Authorized status (i.e. priviledges persists) and 
mPriviledgeOpStarted is false(this is a different job) then set it to true.
- If the user entered the correct password or chose continue proceed with the 
action else halt.


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/CMakeLists.txt, line 37
> > 
> >
> > if (UNIX)
> >add_subdirectory(kauth)
> > endif()
> > 
> > Apparently kauth doesn't exist on Windows.

Shall I move all the kauth specific code to file_unix ?


- Chinmoy Ranjan


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


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 acti

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

[Chinmoy Ranjan Pradhan]

> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.

In my system when i use Shift+del there is no warning dialog (few months back 
dolphin used to show the dialog but strangly it doesn't shows now) so this case 
didn't occured to me. 

If I am not wrong jobuidelegate's warning is shown before the job executes and 
kauth's authentication dialog is shown only after job executes. But its just 
the opposite we want. So if there's a way for slave to notify the job of 
starting of a priviledged action then there should be no problem in making 
jobuidelegate smarter. 
(But the thing is at this very moment i can't think of or find any possible way)


- Chinmoy Ranjan


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


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.

[Chinmoy Ranjan Pradhan]

---
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.


Changes
---

Addressed all the raised issues.


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

D5037: KTextEditor: prevent accidental zooming

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ok, we discard first wheel event cause elapsed timer isn't started, right? I 
think we can "catch" all events we just make it a bit different, set modifiers 
in contructor and restart elpased timer on global event override function if 
desired modifiers presents, so if wheelEvent enters before 200 ms elapsed just 
ignore modifiers so now we don't discard a wheel event just drops earlers 
modifiers.

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

To: rjvbb, #ktexteditor
Cc: anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, 
cullmann, kfunk, sars

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb updated this revision to Diff 12466.
rjvbb added a comment.


  This makes unsetting the modifier non-optional and rewords some comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5037?vs=12448&id=12466

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

AFFECTED FILES
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

To: rjvbb, #ktexteditor
Cc: anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, 
cullmann, kfunk, sars

D5037: KTextEditor: prevent accidental zooming

[René J . V . Bertin]

rjvbb marked an inline comment as done.
rjvbb added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kateviewinternal.cpp:76
> About me, it's designed to unset modifiers so param looks unwanted.

I don't disagree but not for exactly the same reason. I'm not sure the function 
is or should be *designed* for that task. My first reaction to the suggestion 
of creating a helper class was that it would have this somewhat unexpected 
side-effect. That's part of why I added a parameter.

Now the real question is of course if there would ever be use for those 
parameters. I personally see more use for accelerated wheel-scrolling than for 
wheel-zooming but there are probably other ways to trigger it, rather than a 
subtle timing trick.

The modifier argument is less doubtful in my mind; it could be under control of 
a user setting, which would probably even be a prerequisite if someone wants to 
allow accelerated scrolling (also tied to the Control modifier, AFAIK hardwired 
in Qt).

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

To: rjvbb, #ktexteditor
Cc: anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, 
cullmann, kfunk, sars

D4799: Delay notifications until desktop session has loaded

[Valerio Pilo]

vpilo added reviewers: davidedmundson, dfaure, broulik, graesslin, mck182.

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

D5037: KTextEditor: prevent accidental zooming

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kateviewinternal.cpp:76
> +
> +bool detectZoomingEvent(QWheelEvent *e, bool unsetModifier=true, 
> Qt::KeyboardModifiers modifier=Qt::ControlModifier)
> +{

About me, it's designed to unset modifiers so param looks unwanted.

> kateviewinternal.cpp:100
> +// event and produce normal, not accelerated scrolling
> +modState &= ~Qt::ControlModifier;
> +e->setModifiers(modState);

modeState &= ~modifier;

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

To: rjvbb, #ktexteditor
Cc: anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, 
cullmann, kfunk, sars

D5034: Add support for x-gvfs style options in fstab

[Kai Uwe Broulik]

broulik updated this revision to Diff 12457.
broulik added a comment.


  - Cache mountOptions or else it falls out of scope and QStringRef crashes 
(now I know why I should cache it :D)

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5034?vs=12456&id=12457

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/fstab/fstabhandling.cpp
  src/solid/devices/backends/fstab/fstabhandling.h
  src/solid/devices/backends/fstab/fstabstorageaccess.cpp
  src/solid/devices/backends/fstab/fstabstorageaccess.h

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

D5034: Add support for x-gvfs style options in fstab

[Kai Uwe Broulik]

broulik updated this revision to Diff 12456.
broulik edited the summary of this revision.
broulik added a comment.


  - Use QStringRef
  - Drop swap of vendor/product, this will be done in a separate patch

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5034?vs=12435&id=12456

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

AFFECTED FILES
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabdevice.h
  src/solid/devices/backends/fstab/fstabhandling.cpp
  src/solid/devices/backends/fstab/fstabhandling.h
  src/solid/devices/backends/fstab/fstabstorageaccess.cpp
  src/solid/devices/backends/fstab/fstabstorageaccess.h

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

D5034: Add support for x-gvfs style options in fstab

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> broulik wrote in fstabdevice.cpp:40-41
> I wondered that too, that's in the original patch, though, didn't really look 
> into where this will end up, though :D

Just checked, this way round it makes more sense.

The vendor will be the host name and product the path. However, I'll split that 
into a separate patch then.

REPOSITORY
  R245 Solid

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

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

D4508: Plasma controls based on QtQuickControls2

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:76af5399dd95: Plasma controls based on QtQuickControls2 
(authored by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4508?vs=11960&id=12454

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

AFFECTED FILES
  src/declarativeimports/CMakeLists.txt
  src/declarativeimports/plasmacomponents3/BusyIndicator.qml
  src/declarativeimports/plasmacomponents3/Button.qml
  src/declarativeimports/plasmacomponents3/CheckBox.qml
  src/declarativeimports/plasmacomponents3/CheckDelegate.qml
  src/declarativeimports/plasmacomponents3/CheckIndicator.qml
  src/declarativeimports/plasmacomponents3/ComboBox.qml
  src/declarativeimports/plasmacomponents3/Container.qml
  src/declarativeimports/plasmacomponents3/Control.qml
  src/declarativeimports/plasmacomponents3/Dial.qml
  src/declarativeimports/plasmacomponents3/Dialog.qml
  src/declarativeimports/plasmacomponents3/DialogButtonBox.qml
  src/declarativeimports/plasmacomponents3/Drawer.qml
  src/declarativeimports/plasmacomponents3/Frame.qml
  src/declarativeimports/plasmacomponents3/GroupBox.qml
  src/declarativeimports/plasmacomponents3/ItemDelegate.qml
  src/declarativeimports/plasmacomponents3/Label.qml
  src/declarativeimports/plasmacomponents3/Menu.qml
  src/declarativeimports/plasmacomponents3/MenuItem.qml
  src/declarativeimports/plasmacomponents3/Popup.qml
  src/declarativeimports/plasmacomponents3/ProgressBar.qml
  src/declarativeimports/plasmacomponents3/RadioButton.qml
  src/declarativeimports/plasmacomponents3/RadioDelegate.qml
  src/declarativeimports/plasmacomponents3/RadioIndicator.qml
  src/declarativeimports/plasmacomponents3/RangeSlider.qml
  src/declarativeimports/plasmacomponents3/ScrollBar.qml
  src/declarativeimports/plasmacomponents3/Slider.qml
  src/declarativeimports/plasmacomponents3/SpinBox.qml
  src/declarativeimports/plasmacomponents3/Switch.qml
  src/declarativeimports/plasmacomponents3/SwitchDelegate.qml
  src/declarativeimports/plasmacomponents3/SwitchIndicator.qml
  src/declarativeimports/plasmacomponents3/TabBar.qml
  src/declarativeimports/plasmacomponents3/TabButton.qml
  src/declarativeimports/plasmacomponents3/TextArea.qml
  src/declarativeimports/plasmacomponents3/TextField.qml
  src/declarativeimports/plasmacomponents3/ToolBar.qml
  src/declarativeimports/plasmacomponents3/ToolButton.qml
  src/declarativeimports/plasmacomponents3/ToolTip.qml
  src/declarativeimports/plasmacomponents3/private/ButtonShadow.qml
  src/declarativeimports/plasmacomponents3/private/DefaultListItemBackground.qml
  src/declarativeimports/plasmacomponents3/private/RoundShadow.qml
  src/declarativeimports/plasmacomponents3/private/TextFieldFocus.qml
  src/declarativeimports/plasmacomponents3/qmldir

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