[RFC] KCM Launch Feedback not saving cursor settings
ClosedPublic

Authored by sharvey on Mar 20 2018, 6:20 PM.

Details

Summary

Launch Feedback KCM was not saving the cursor notifications properly - Apply button would not activate, klaunchrc was not being updated. QML was using calls to Qt.later() before setting the radio button index, generating "unknown property" errors from QML and not emitting the change signal. Simply removed those calls and the QML functions properly, and klaunchrc is updated correctly.

Per @davidedmundson, Qt.later() may be too new of a function to be supported by Plasma.

NOTE: I was getting occasional and random segmentation faults (missing shared library?) while testing this fix. I'm reasonably certain my environment is at fault, but I need another pair of experienced eyes to investigate. I'll write a suitable test plan once that's resolved.

BUG: 392050

Diff Detail

Repository
R119 Plasma Desktop
Branch
kcm-launchfeedback-cursor (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
sharvey created this revision.Mar 20 2018, 6:20 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 20 2018, 6:20 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sharvey requested review of this revision.Mar 20 2018, 6:20 PM
sharvey edited the summary of this revision. (Show Details)Mar 20 2018, 6:30 PM
sharvey added a subscriber: davidedmundson.

Thanks for the patch! Formatting-wise, please capitalize Bug (should be BUG).

kcms/launch/package/contents/ui/main.qml
63–64

You can now remove the braces and semicolon here (and in the other instances).

sharvey edited the summary of this revision. (Show Details)Mar 20 2018, 6:34 PM
sharvey updated this revision to Diff 30031.Mar 20 2018, 6:42 PM
sharvey edited the summary of this revision. (Show Details)
  • Removed erroneous braces and semicolons
sharvey marked an inline comment as done.Mar 20 2018, 6:46 PM

Braces and semicolons removed. Taking these away was one of the ways I'd get a segfault. This time, however, all's well in the world. I need to keep debugging my environment.

hein added a comment.Mar 20 2018, 7:14 PM

The actuall bug here is pretty stupid - the function is called callLater(), not later(). Could you modify the patch to use callLater?

In D11522#230217, @hein wrote:

The actuall bug here is pretty stupid - the function is called callLater(), not later(). Could you modify the patch to use callLater?

Sure, easy enough. As long as you (or anyone, really) explains why we hold off on triggering this until the next event loop. Is it an optimization technique?

sharvey updated this revision to Diff 30042.Mar 20 2018, 7:46 PM
  • Revise code to use Qt.callLater() instead of invalid Qt.later()

Per @hein, revised with correct function call

hein accepted this revision.Mar 21 2018, 2:26 AM

Nah, it's to prevent an infinite loop between the different radio buttons and the backend object, since onCheckedChange will fire for both false and true.

Thanks! Should I commit this for you? :)

This revision is now accepted and ready to land.Mar 21 2018, 2:26 AM
In D11522#230474, @hein wrote:

Nah, it's to prevent an infinite loop between the different radio buttons and the backend object, since onCheckedChange will fire for both false and true.

Makes sense, in a QML kind of way.

Thanks! Should I commit this for you? :)

If you'd be so kind. Glad to help. :)

This revision was automatically updated to reflect the committed changes.