Correct inital loading of BorderActivate
ClosedPublic

Authored by davidedmundson on Jan 13 2017, 9:03 AM.

Details

Summary

We have a comma separated list:

"".split(',') returns ""
JS decides that's worth itterating over, and we implicitly cast "" to a
number which means we register on screen edge 0 (the top).

We can't use typeof is isFinite because valid entries are still strings
at this point.

Test Plan

Ran with default config, no longer registered on top edge
Set an edge properly in the KCM. Still worked.

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.
davidedmundson retitled this revision from to Correct inital loading of BorderActivate.
davidedmundson updated this object.
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: KWin. · View Herald TranscriptJan 13 2017, 9:03 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
mart accepted this revision.Jan 13 2017, 3:08 PM
mart added a reviewer: mart.
mart added a subscriber: mart.

isthere a bug # for this?

This revision is now accepted and ready to land.Jan 13 2017, 3:08 PM

isthere a bug # for this?

There's a Reddit comment...the new bugzilla.

This revision was automatically updated to reflect the committed changes.

isthere a bug # for this?

There's a Reddit comment...the new bugzilla.

Maybe we need a new bugzilla hook which can automatically reply to reddit threads.

The entire parsing is totally not safe against JoeReddiot murking around in the config file, I wonder what happens if we pass "-1" and what is " " cast as...

One should probably use "parseInt(border, 10);"?

davidedmundson reopened this revision.Jan 17 2017, 1:03 PM

I wonder what happens if we pass "-1" and what is " " cast as...

If you pass -1, then registerScreenEdge would reject it, and " " would cast as 0.
So absolutely worst case you get an extra border, nothing too bad.

However, you're right that validation on the JS side would be cleaner. Will do that.

This revision is now accepted and ready to land.Jan 17 2017, 1:03 PM
davidedmundson edited edge metadata.

Validate on JS side

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptJan 17 2017, 1:04 PM
Restricted Application added a subscriber: KWin. · View Herald Transcript
luebking accepted this revision.Jan 17 2017, 1:43 PM
luebking added a reviewer: luebking.
This revision was automatically updated to reflect the committed changes.