[kcmkwin/kwindesktop] Fix inability to create more than one row on the "Virtual Desktops" settings page
ClosedPublic

Authored by epopov on Mar 28 2020, 4:00 PM.

Details

Summary

When a spinbox with the number of rows loses focus, the value of this spinbox (i.e., the count of rows) is always reset to 1. Thus, it's impossible to create more then one row.

To fix this bug, we need to implement valueFromText function.

BUG: 419141
FIXED-IN: 5.18.4

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
epopov created this revision.Mar 28 2020, 4:00 PM
Restricted Application added a project: KWin. · View Herald TranscriptMar 28 2020, 4:00 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
epopov requested review of this revision.Mar 28 2020, 4:00 PM
davidedmundson accepted this revision.Mar 28 2020, 4:17 PM
This revision is now accepted and ready to land.Mar 28 2020, 4:17 PM
ngraham accepted this revision.Mar 30 2020, 12:13 AM
ngraham retitled this revision from Bug 419141: Cannot create more than one row on the "Virtual Desktops" settings page to [kcmkwin/kwindesktop] Fix inability to create more than one row on the "Virtual Desktops" settings page.
ngraham edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
zzag added a comment.EditedMar 30 2020, 11:24 AM

Shouldn't we use locale-aware functions for parsing numbers? (if there's any)

There's no easy way to do this. I can use something like this:

return Number.fromLocaleString(locale, text.match(new RegExp("[" + locale.groupSeparator + "\\d]+")))

But currently, the number of rows is a positive integer in the range [1..20], so we don't need to take into account the locale when parsing it.

zzag added a comment.Mar 30 2020, 1:51 PM

There's no easy way to do this.

Agreed.

But currently, the number of rows is a positive integer in the range [1..20], so we don't need to take into account the locale when parsing it.

Unfortunately, that's not the case. See D24838#570122

Well then I have no idea how to fix it

zzag added a subscriber: broulik.Mar 31 2020, 9:09 AM

Yeah, it is a bit tricky problem and affects not only kwin, but other projects as well. @davidedmundson @broulik Have you encountered this problem before in plasma shell? if so, did you come up with a solution that we could re-use here in kwin?