Merge lp:~mterry/unity8/greeter-focus into lp:unity8
- greeter-focus
- Merge into trunk
| Status: | Superseded |
|---|---|
| Proposed branch: | lp:~mterry/unity8/greeter-focus |
| Merge into: | lp:unity8 |
| Prerequisite: | lp:~josharenson/unity8/sessions-model |
| Diff against target: |
808 lines (+237/-91) 13 files modified
qml/Greeter/Greeter.qml (+27/-23) qml/Greeter/GreeterPrompt.qml (+68/-10) qml/Greeter/LoginList.qml (+27/-16) qml/Greeter/NarrowView.qml (+2/-1) qml/Greeter/WideView.qml (+4/-3) qml/Shell.qml (+5/-3) tests/qmltests/Greeter/TestView.qml (+0/-1) tests/qmltests/Greeter/tst_Greeter.qml (+2/-12) tests/qmltests/Greeter/tst_WideView.qml (+90/-12) tests/qmltests/Tutorial/tst_Tutorial.qml (+1/-1) tests/qmltests/tst_OrientedShell.qml (+2/-2) tests/qmltests/tst_Shell.qml (+3/-3) tests/qmltests/tst_ShellWithPin.qml (+6/-4) |
| To merge this branch: | bzr merge lp:~mterry/unity8/greeter-focus |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Unity8 CI Bot | continuous-integration | Needs Fixing | |
| Daniel d'Andrada (community) | Needs Fixing | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2016-06-22.
This proposal has been superseded by a proposal from 2016-07-06.
Commit message
Fix tablet greeter focus to be always-on, no longer hiding the OSK or forcing a mouse click to type a password.
I've also squeezed some other small fixes in:
- Support Up and Down keys in user list.
- Don't accept non-digit characters when prompting for passcode (we ask OSK to show only digits, but due to bug 1586435, it shows other characters too; plus if the user has an external keyboard, they can type anything anyway).
- When prompting for a passcode in wide-view (tablet/desktop), stop at 4 digits just like we do in narrow-view.
- Don't show "Retry" during brief period before a prompt comes in from PAM.
- Don't let user drag user list if there's only one entry.
Now as for the focus changes...
One of the big reasons we lost keyboard focus before was because we set the shell to disabled whenever the greeter was "between PAM events" -- mostly so that the user can't swipe away greeter before we get our first prompt or are otherwise unsure about exactly what authentication is needed.
But disabled qml items can't be focused. So I've rejiggered things a bit. Instead of disabling the whole shell, I just disable the launcher, the indicators, and make the greeter non-swipeable.
I also do some tricks with the prompt so that it looks disabled (while we check with PAM about the password) but isn't actually. I don't want to make it look to the user that pressing backspace while they wait for PAM does anything, so after the user presses Enter, I present a fake label on top of the prompt that looks like a disabled prompt, while simultaneously hiding the prompt contents.
Description of the change
You could mostly just test this on your desktop. Changes are all to the wide-view side of things.
* Are there any related MPs required for this MP to build/function as expected? Please list.
No
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
NA
* If you changed the UI, has there been a design review?
NA
| Michael Terry (mterry) wrote : Posted in a previous version of this proposal | # |
| Michael Terry (mterry) wrote : Posted in a previous version of this proposal | # |
OK, tests added.
| Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:2496
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:2497
https:/
Executed test runs:
SUCCESS: https:/
FAILURE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:2498
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:1999
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Daniel d'Andrada (dandrader) wrote : | # |
"""
+ // Oddly, a simple "focus: visible" does not stick -- the binding
187 + // gets lost when switching between buttons and prompts. But this
188 + // explicit binding does work.
189 + Binding on focus {
190 + value: passwordInput.
191 + }
"""
It doesn't stick because the focus property gets written directly by someone else. So your biding gets overwritten by a direct assigment. A Binding{} element doesn't share this fate since it's kept separately and works differently. Everytime it's value expression gets reevaluated it makes a direct assigment to its target (or so I believe that's how it works).
Overall I think it's a bad practice to have a binding on the focus property. Qt itself may change the focus property of an item at any time as the result of some other item in the same focus scope having focus.
So for focus setting you either have imperative code like this:
onVisibleChanged: {
focus = true;
}
Or better yet, evaluate if adding a FocusScope somewhere in your item hierarchy wouldn't solve that "mystery". As focus changes outside your FocusScope wouldn't change the focus inside it.
| Daniel d'Andrada (dandrader) wrote : | # |
"""
Overall I think it's a bad practice to have a binding on the focus property. Qt itself may change the focus property of an item at any time as the result of some other item in the same focus scope having focus.
"""
Let me rephrase that part: It's a bad practice if you don't have full control over the focus scope of that specific item.
- 2000. By Michael Terry
-
Be smoother about focus in GreeterPrompt
| Michael Terry (mterry) wrote : | # |
I adjusted it to be imperative instead of declarative. Thanks for breaking Qt focus down for me here and on IRC. :)
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2000
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2001. By Michael Terry
-
remove unneeded check, fixes test
- 2002. By Michael Terry
-
Merge in greeter-
hide-indicators
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2001
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2003. By Michael Terry
-
Update copyrights
- 2004. By Michael Terry
-
Give wideview test focus to start
- 2005. By Michael Terry
-
Merge focus refactor from dandrader
- 2006. By Michael Terry
-
Merge tryWideView focus refactor from dandrader
- 2007. By Michael Terry
-
Make it clear in GreeterPrompt that fakeLabel is for OSK focus
- 2008. By Michael Terry
-
Fix a test
Unmerged revisions
Preview Diff
| 1 | === modified file 'qml/Greeter/Greeter.qml' |
| 2 | --- qml/Greeter/Greeter.qml 2016-07-06 18:42:19 +0000 |
| 3 | +++ qml/Greeter/Greeter.qml 2016-07-06 18:42:21 +0000 |
| 4 | @@ -163,31 +163,37 @@ |
| 5 | return -1; |
| 6 | } |
| 7 | |
| 8 | - function selectUser(uid, reset) { |
| 9 | - if (uid < 0) |
| 10 | + function selectUser(index, reset) { |
| 11 | + if (index < 0 || index >= LightDMService.users.count) |
| 12 | return; |
| 13 | d.waiting = true; |
| 14 | if (reset) { |
| 15 | loader.item.reset(); |
| 16 | } |
| 17 | - currentIndex = uid; |
| 18 | - var user = LightDMService.users.data(uid, LightDMService.userRoles.NameRole); |
| 19 | + currentIndex = index; |
| 20 | + var user = LightDMService.users.data(index, LightDMService.userRoles.NameRole); |
| 21 | AccountsService.user = user; |
| 22 | LauncherModel.setUser(user); |
| 23 | LightDMService.greeter.authenticate(user); // always resets auth state |
| 24 | } |
| 25 | |
| 26 | + function hideView() { |
| 27 | + if (loader.item) { |
| 28 | + loader.item.enabled = false; // drop OSK and prevent interaction |
| 29 | + loader.item.notifyAuthenticationSucceeded(false /* showFakePassword */); |
| 30 | + loader.item.hide(); |
| 31 | + } |
| 32 | + } |
| 33 | + |
| 34 | function login() { |
| 35 | - enabled = false; |
| 36 | + d.waiting = true; |
| 37 | if (LightDMService.greeter.startSessionSync()) { |
| 38 | sessionStarted(); |
| 39 | - if (loader.item) { |
| 40 | - loader.item.notifyAuthenticationSucceeded(false /* showFakePassword */); |
| 41 | - } |
| 42 | + hideView(); |
| 43 | } else if (loader.item) { |
| 44 | loader.item.notifyAuthenticationFailed(); |
| 45 | } |
| 46 | - enabled = true; |
| 47 | + d.waiting = false; |
| 48 | } |
| 49 | |
| 50 | function startUnlock(toTheRight) { |
| 51 | @@ -199,10 +205,8 @@ |
| 52 | } |
| 53 | |
| 54 | function checkForcedUnlock(hideNow) { |
| 55 | - if (forcedUnlock && shown && loader.item) { |
| 56 | - // pretend we were just authenticated |
| 57 | - loader.item.notifyAuthenticationSucceeded(false /* showFakePassword */); |
| 58 | - loader.item.hide(); |
| 59 | + if (forcedUnlock && shown) { |
| 60 | + hideView(); |
| 61 | if (hideNow) { |
| 62 | root.hideNow(); // skip hide animation |
| 63 | } |
| 64 | @@ -323,7 +327,7 @@ |
| 65 | |
| 66 | onLoaded: { |
| 67 | root.lockedApp = ""; |
| 68 | - root.forceActiveFocus(); |
| 69 | + item.forceActiveFocus(); |
| 70 | d.selectUser(d.currentIndex, true); |
| 71 | LightDMService.infographic.readyForDataChange(); |
| 72 | } |
| 73 | @@ -333,13 +337,11 @@ |
| 74 | onSelected: { |
| 75 | d.selectUser(index, true); |
| 76 | } |
| 77 | - onPromptlessLogin: d.login(); |
| 78 | onResponded: { |
| 79 | if (root.locked) { |
| 80 | LightDMService.greeter.respond(response); |
| 81 | } else { |
| 82 | d.login(); |
| 83 | - loader.item.hide(); |
| 84 | } |
| 85 | } |
| 86 | onTease: root.tease() |
| 87 | @@ -389,6 +391,12 @@ |
| 88 | |
| 89 | Binding { |
| 90 | target: loader.item |
| 91 | + property: "waiting" |
| 92 | + value: d.waiting |
| 93 | + } |
| 94 | + |
| 95 | + Binding { |
| 96 | + target: loader.item |
| 97 | property: "alphanumeric" |
| 98 | value: d.alphanumeric |
| 99 | } |
| 100 | @@ -417,10 +425,7 @@ |
| 101 | |
| 102 | onShowGreeter: root.forceShow() |
| 103 | |
| 104 | - onHideGreeter: { |
| 105 | - d.login(); |
| 106 | - loader.item.hide(); |
| 107 | - } |
| 108 | + onHideGreeter: d.login() |
| 109 | |
| 110 | onShowMessage: { |
| 111 | // inefficient, but we only rarely deal with messages |
| 112 | @@ -438,11 +443,11 @@ |
| 113 | } |
| 114 | |
| 115 | onShowPrompt: { |
| 116 | - d.waiting = false; |
| 117 | - |
| 118 | if (loader.item) { |
| 119 | loader.item.showPrompt(text, isSecret, isDefaultPrompt); |
| 120 | } |
| 121 | + |
| 122 | + d.waiting = false; |
| 123 | } |
| 124 | |
| 125 | onAuthenticationComplete: { |
| 126 | @@ -451,7 +456,6 @@ |
| 127 | if (LightDMService.greeter.authenticated) { |
| 128 | if (!LightDMService.greeter.promptless) { |
| 129 | d.login(); |
| 130 | - loader.item.hide(); |
| 131 | } |
| 132 | } else { |
| 133 | if (!LightDMService.greeter.promptless) { |
| 134 | |
| 135 | === modified file 'qml/Greeter/GreeterPrompt.qml' |
| 136 | --- qml/Greeter/GreeterPrompt.qml 2016-06-06 18:21:22 +0000 |
| 137 | +++ qml/Greeter/GreeterPrompt.qml 2016-07-06 18:42:21 +0000 |
| 138 | @@ -16,6 +16,7 @@ |
| 139 | |
| 140 | import QtQuick 2.4 |
| 141 | import Ubuntu.Components 1.3 |
| 142 | +import "../Components" |
| 143 | |
| 144 | FocusScope { |
| 145 | id: root |
| 146 | @@ -33,6 +34,7 @@ |
| 147 | |
| 148 | function reset() { |
| 149 | passwordInput.text = ""; |
| 150 | + fakeLabel.text = ""; |
| 151 | d.enabled = true; |
| 152 | } |
| 153 | |
| 154 | @@ -62,10 +64,20 @@ |
| 155 | objectName: "promptButton" |
| 156 | anchors.fill: parent |
| 157 | visible: !root.isPrompt |
| 158 | - enabled: d.enabled |
| 159 | focus: visible |
| 160 | |
| 161 | - onClicked: root.clicked() |
| 162 | + onVisibleChanged: { |
| 163 | + // Qt owns focus, we can't keep our binding active. So make sure |
| 164 | + // focus stays in sync manually. |
| 165 | + focus = visible; |
| 166 | + } |
| 167 | + |
| 168 | + onClicked: { |
| 169 | + if (d.enabled) { |
| 170 | + d.enabled = false; |
| 171 | + root.clicked(); |
| 172 | + } |
| 173 | + } |
| 174 | |
| 175 | Label { |
| 176 | anchors.centerIn: parent |
| 177 | @@ -79,10 +91,22 @@ |
| 178 | objectName: "promptField" |
| 179 | anchors.fill: parent |
| 180 | visible: root.isPrompt |
| 181 | - enabled: d.enabled |
| 182 | + opacity: fakeLabel.visible ? 0 : 1 |
| 183 | focus: visible |
| 184 | |
| 185 | - inputMethodHints: root.isAlphanumeric ? Qt.ImhNone : Qt.ImhDigitsOnly |
| 186 | + onVisibleChanged: { |
| 187 | + // Qt owns focus, we can't keep our binding active. So make sure |
| 188 | + // focus stays in sync manually. |
| 189 | + focus = visible; |
| 190 | + } |
| 191 | + |
| 192 | + validator: RegExpValidator { |
| 193 | + regExp: root.isAlphanumeric ? /^.*$/ : /^\d{4}$/ |
| 194 | + } |
| 195 | + |
| 196 | + inputMethodHints: Qt.ImhSensitiveData | Qt.ImhNoPredictiveText | |
| 197 | + Qt.ImhMultiLine | // so OSK doesn't close on Enter |
| 198 | + (root.isAlphanumeric ? Qt.ImhNone : Qt.ImhDigitsOnly) |
| 199 | echoMode: root.isSecret ? TextInput.Password : TextInput.Normal |
| 200 | hasClearButton: false |
| 201 | |
| 202 | @@ -107,17 +131,35 @@ |
| 203 | width: units.gu(3) |
| 204 | color: d.textColor |
| 205 | visible: root.isSecret && false // TODO: detect when caps lock is on |
| 206 | + readonly property real visibleWidth: visible ? width + passwordInput.frameSpacing : 0 |
| 207 | } |
| 208 | ] |
| 209 | |
| 210 | + onDisplayTextChanged: { |
| 211 | + // We use onDisplayTextChanged instead of onTextChanged because |
| 212 | + // displayText changes after text and if we did this before it |
| 213 | + // updated, we would use the wrong displayText for fakeLabel. |
| 214 | + if (!isAlphanumeric && text.length >= 4) { |
| 215 | + // hard limit of 4 for passcodes right now |
| 216 | + respond(); |
| 217 | + } |
| 218 | + } |
| 219 | + |
| 220 | onAccepted: { |
| 221 | - if (!enabled) |
| 222 | - return; |
| 223 | + if (d.enabled) |
| 224 | + respond(); |
| 225 | + } |
| 226 | + |
| 227 | + function respond() { |
| 228 | d.enabled = false; |
| 229 | + fakeLabel.text = displayText; |
| 230 | root.responded(text); |
| 231 | } |
| 232 | |
| 233 | - Keys.onEscapePressed: root.canceled() |
| 234 | + Keys.onEscapePressed: { |
| 235 | + root.canceled(); |
| 236 | + event.accepted = true; |
| 237 | + } |
| 238 | |
| 239 | // We use our own custom placeholder label instead of the standard |
| 240 | // TextField one because the standard one hardcodes baseText as the |
| 241 | @@ -129,9 +171,7 @@ |
| 242 | right: parent.right |
| 243 | verticalCenter: parent.verticalCenter |
| 244 | leftMargin: units.gu(1.5) |
| 245 | - rightMargin: anchors.leftMargin + |
| 246 | - (capsIcon.visible ? capsIcon.width + passwordInput.frameSpacing |
| 247 | - : 0) |
| 248 | + rightMargin: anchors.leftMargin + capsIcon.visibleWidth |
| 249 | } |
| 250 | text: root.text |
| 251 | visible: passwordInput.text == "" && !passwordInput.inputMethodComposing |
| 252 | @@ -139,4 +179,22 @@ |
| 253 | elide: Text.ElideRight |
| 254 | } |
| 255 | } |
| 256 | + |
| 257 | + // Have a fake label that covers the text field after the user presses |
| 258 | + // enter. What we *really* want is a disabled mode that doesn't lose |
| 259 | + // focus. Because our goal here is simply to keep the OSK up while |
| 260 | + // we wait for PAM to get back to us, and while waiting, we don't want |
| 261 | + // the user to be able to edit the field (simply because it would look |
| 262 | + // weird if we allowed that). But until we have such a disabled mode, |
| 263 | + // we'll fake it by covering the real text field with a label. |
| 264 | + FadingLabel { |
| 265 | + id: fakeLabel |
| 266 | + anchors.verticalCenter: parent.verticalCenter |
| 267 | + anchors.left: parent.left |
| 268 | + anchors.right: parent.right |
| 269 | + anchors.leftMargin: passwordInput.frameSpacing * 2 |
| 270 | + anchors.rightMargin: passwordInput.frameSpacing * 2 + capsIcon.visibleWidth |
| 271 | + color: d.drawColor |
| 272 | + visible: root.isPrompt && !d.enabled |
| 273 | + } |
| 274 | } |
| 275 | |
| 276 | === modified file 'qml/Greeter/LoginList.qml' |
| 277 | --- qml/Greeter/LoginList.qml 2016-07-06 18:42:19 +0000 |
| 278 | +++ qml/Greeter/LoginList.qml 2016-07-06 18:42:21 +0000 |
| 279 | @@ -21,11 +21,13 @@ |
| 280 | |
| 281 | StyledItem { |
| 282 | id: root |
| 283 | + focus: true |
| 284 | |
| 285 | property alias model: userList.model |
| 286 | property bool alphanumeric: true |
| 287 | property int currentIndex |
| 288 | property bool locked |
| 289 | + property bool waiting |
| 290 | |
| 291 | readonly property int numAboveBelow: 4 |
| 292 | readonly property int cellHeight: units.gu(5) |
| 293 | @@ -36,7 +38,6 @@ |
| 294 | |
| 295 | signal selected(int index) |
| 296 | signal responded(string response) |
| 297 | - signal promptlessLogin() |
| 298 | |
| 299 | function tryToUnlock() { |
| 300 | if (wasPrompted) { |
| 301 | @@ -45,7 +46,6 @@ |
| 302 | if (root.locked) { |
| 303 | root.selected(currentIndex); |
| 304 | } else { |
| 305 | - promptlessLogin(); |
| 306 | root.responded(""); |
| 307 | } |
| 308 | } |
| 309 | @@ -60,20 +60,18 @@ |
| 310 | } |
| 311 | |
| 312 | function showPrompt(text, isSecret, isDefaultPrompt) { |
| 313 | - d.promptText = text; |
| 314 | - passwordInput.reset(); |
| 315 | + passwordInput.text = isDefaultPrompt ? alphanumeric ? i18n.tr("Passphrase") |
| 316 | + : i18n.tr("Passcode") |
| 317 | + : text; |
| 318 | + passwordInput.isPrompt = true; |
| 319 | passwordInput.isSecret = isSecret; |
| 320 | - if (wasPrompted) // stay in text field if second prompt |
| 321 | - passwordInput.focus = true; |
| 322 | + passwordInput.reset(); |
| 323 | wasPrompted = true; |
| 324 | } |
| 325 | |
| 326 | function showError() { |
| 327 | wrongPasswordAnimation.start(); |
| 328 | root.resetAuthentication(); |
| 329 | - if (wasPrompted) { |
| 330 | - passwordInput.focus = true; |
| 331 | - } |
| 332 | } |
| 333 | |
| 334 | function reset() { |
| 335 | @@ -83,15 +81,33 @@ |
| 336 | QtObject { |
| 337 | id: d |
| 338 | |
| 339 | - property string promptText |
| 340 | + function checkIfPromptless() { |
| 341 | + if (!waiting && !wasPrompted) { |
| 342 | + passwordInput.isPrompt = false; |
| 343 | + passwordInput.text = root.locked ? i18n.tr("Retry") |
| 344 | + : i18n.tr("Log In") |
| 345 | + } |
| 346 | + } |
| 347 | } |
| 348 | |
| 349 | + onWaitingChanged: d.checkIfPromptless() |
| 350 | + onLockedChanged: d.checkIfPromptless() |
| 351 | + |
| 352 | theme: ThemeSettings { |
| 353 | name: "Ubuntu.Components.Themes.Ambiance" |
| 354 | } |
| 355 | |
| 356 | + Keys.onUpPressed: { |
| 357 | + selected(currentIndex - 1); |
| 358 | + event.accepted = true; |
| 359 | + } |
| 360 | + Keys.onDownPressed: { |
| 361 | + selected(currentIndex + 1); |
| 362 | + event.accepted = true; |
| 363 | + } |
| 364 | Keys.onEscapePressed: { |
| 365 | selected(currentIndex); |
| 366 | + event.accepted = true; |
| 367 | } |
| 368 | |
| 369 | onCurrentIndexChanged: { |
| 370 | @@ -137,6 +153,7 @@ |
| 371 | highlightRangeMode: ListView.StrictlyEnforceRange |
| 372 | highlightMoveDuration: root.moveDuration |
| 373 | flickDeceleration: 10000 |
| 374 | + interactive: count > 1 |
| 375 | |
| 376 | readonly property bool movingInternally: moveTimer.running || userList.moving |
| 377 | onMovingInternallyChanged: { |
| 378 | @@ -254,13 +271,8 @@ |
| 379 | width: highlightItem.width - anchors.margins * 2 |
| 380 | opacity: userList.movingInternally ? 0 : 1 |
| 381 | |
| 382 | - isPrompt: root.wasPrompted |
| 383 | isAlphanumeric: root.alphanumeric |
| 384 | |
| 385 | - text: root.wasPrompted ? d.promptText |
| 386 | - : (root.locked ? i18n.tr("Retry") |
| 387 | - : i18n.tr("Log In")) |
| 388 | - |
| 389 | onClicked: root.tryToUnlock() |
| 390 | onResponded: root.responded(text) |
| 391 | onCanceled: root.selected(currentIndex) |
| 392 | @@ -280,7 +292,6 @@ |
| 393 | return; |
| 394 | } |
| 395 | infoLabel.text = ""; |
| 396 | - d.promptText = ""; |
| 397 | passwordInput.reset(); |
| 398 | root.wasPrompted = false; |
| 399 | } |
| 400 | |
| 401 | === modified file 'qml/Greeter/NarrowView.qml' |
| 402 | --- qml/Greeter/NarrowView.qml 2016-06-20 20:23:28 +0000 |
| 403 | +++ qml/Greeter/NarrowView.qml 2016-07-06 18:42:21 +0000 |
| 404 | @@ -31,6 +31,7 @@ |
| 405 | property bool alphanumeric |
| 406 | property var userModel // unused |
| 407 | property alias infographicModel: coverPage.infographicModel |
| 408 | + property bool waiting |
| 409 | readonly property bool fullyShown: coverPage.showProgress === 1 || lockscreen.shown |
| 410 | readonly property bool required: coverPage.required || lockscreen.required |
| 411 | readonly property bool animating: coverPage.showAnimation.running || coverPage.hideAnimation.running |
| 412 | @@ -71,7 +72,6 @@ |
| 413 | if (!alphanumeric && showFakePassword) { |
| 414 | lockscreen.showText("...."); // actual text doesn't matter, we show bullets |
| 415 | } |
| 416 | - lockscreen.hide(); |
| 417 | } |
| 418 | |
| 419 | function notifyAuthenticationFailed() { |
| 420 | @@ -172,6 +172,7 @@ |
| 421 | height: parent.height |
| 422 | width: parent.width |
| 423 | background: root.background |
| 424 | + draggable: !root.waiting |
| 425 | onTease: root.tease() |
| 426 | onClicked: hide() |
| 427 | |
| 428 | |
| 429 | === modified file 'qml/Greeter/WideView.qml' |
| 430 | --- qml/Greeter/WideView.qml 2016-07-06 18:42:19 +0000 |
| 431 | +++ qml/Greeter/WideView.qml 2016-07-06 18:42:21 +0000 |
| 432 | @@ -19,6 +19,7 @@ |
| 433 | |
| 434 | FocusScope { |
| 435 | id: root |
| 436 | + focus: true |
| 437 | |
| 438 | property alias dragHandleLeftMargin: coverPage.dragHandleLeftMargin |
| 439 | property alias launcherOffset: coverPage.launcherOffset |
| 440 | @@ -30,6 +31,7 @@ |
| 441 | property alias alphanumeric: loginList.alphanumeric |
| 442 | property alias userModel: loginList.model |
| 443 | property alias infographicModel: coverPage.infographicModel |
| 444 | + property bool waiting |
| 445 | readonly property bool fullyShown: coverPage.showProgress === 1 |
| 446 | readonly property bool required: coverPage.required |
| 447 | readonly property bool animating: coverPage.showAnimation.running || coverPage.hideAnimation.running |
| 448 | @@ -37,7 +39,6 @@ |
| 449 | // so that it can be replaced in tests with a mock object |
| 450 | property var inputMethod: Qt.inputMethod |
| 451 | |
| 452 | - signal promptlessLogin() |
| 453 | signal selected(int index) |
| 454 | signal responded(string response) |
| 455 | signal tease() |
| 456 | @@ -102,7 +103,7 @@ |
| 457 | objectName: "coverPage" |
| 458 | height: parent.height |
| 459 | width: parent.width |
| 460 | - draggable: !root.locked |
| 461 | + draggable: !root.locked && !root.waiting |
| 462 | |
| 463 | infographics { |
| 464 | height: 0.75 * parent.height |
| 465 | @@ -132,8 +133,8 @@ |
| 466 | Behavior on height { UbuntuNumberAnimation {} } |
| 467 | |
| 468 | locked: root.locked |
| 469 | + waiting: root.waiting |
| 470 | |
| 471 | - onPromptlessLogin: root.promptlessLogin() |
| 472 | onSelected: root.selected(index) |
| 473 | onResponded: root.responded(response) |
| 474 | } |
| 475 | |
| 476 | === modified file 'qml/Shell.qml' |
| 477 | --- qml/Shell.qml 2016-07-06 18:42:19 +0000 |
| 478 | +++ qml/Shell.qml 2016-07-06 18:42:21 +0000 |
| 479 | @@ -137,9 +137,9 @@ |
| 480 | // For autopilot consumption |
| 481 | readonly property string focusedApplicationId: ApplicationManager.focusedApplicationId |
| 482 | |
| 483 | - // Disable everything while greeter is waiting, so that the user can't swipe |
| 484 | - // the greeter or launcher until we know whether the session is locked. |
| 485 | - enabled: greeter && !greeter.waiting |
| 486 | + // Note when greeter is waiting on PAM, so that we can disable edges until |
| 487 | + // we know which user data to show and whether the session is locked. |
| 488 | + readonly property bool waitingOnGreeter: greeter && greeter.waiting |
| 489 | |
| 490 | property real edgeSize: units.gu(settings.edgeDragWidth) |
| 491 | |
| 492 | @@ -533,6 +533,7 @@ |
| 493 | available: tutorial.panelEnabled |
| 494 | && ((!greeter || !greeter.locked) || AccountsService.enableIndicatorsWhileLocked) |
| 495 | && (!greeter || !greeter.hasLockedApp) |
| 496 | + && !shell.waitingOnGreeter |
| 497 | width: parent.width > units.gu(60) ? units.gu(40) : parent.width |
| 498 | |
| 499 | minimizedPanelHeight: units.gu(3) |
| 500 | @@ -583,6 +584,7 @@ |
| 501 | available: tutorial.launcherEnabled |
| 502 | && (!greeter.locked || AccountsService.enableLauncherWhileLocked) |
| 503 | && !greeter.hasLockedApp |
| 504 | + && !shell.waitingOnGreeter |
| 505 | inverted: shell.usageScenario !== "desktop" |
| 506 | superPressed: physicalKeysMapper.superPressed |
| 507 | superTabPressed: physicalKeysMapper.superTabPressed |
| 508 | |
| 509 | === modified file 'tests/qmltests/Greeter/TestView.qml' |
| 510 | --- tests/qmltests/Greeter/TestView.qml 2016-07-06 18:42:19 +0000 |
| 511 | +++ tests/qmltests/Greeter/TestView.qml 2016-05-25 19:25:51 +0000 |
| 512 | @@ -43,7 +43,6 @@ |
| 513 | signal responded(string response) |
| 514 | signal tease() |
| 515 | signal emergencyCall() |
| 516 | - signal promptlessLogin() |
| 517 | |
| 518 | signal _showMessageCalled(string html) |
| 519 | signal _showPromptCalled(string text, bool isSecret, bool isDefaultPrompt) |
| 520 | |
| 521 | === modified file 'tests/qmltests/Greeter/tst_Greeter.qml' |
| 522 | --- tests/qmltests/Greeter/tst_Greeter.qml 2016-07-06 18:42:19 +0000 |
| 523 | +++ tests/qmltests/Greeter/tst_Greeter.qml 2016-07-06 18:42:21 +0000 |
| 524 | @@ -194,6 +194,8 @@ |
| 525 | var i = getIndexOf(name); |
| 526 | view.selected(i); |
| 527 | verifySelected(name); |
| 528 | + compare(greeter.waiting, true); |
| 529 | + tryCompare(greeter, "waiting", false); |
| 530 | return i; |
| 531 | } |
| 532 | |
| 533 | @@ -325,18 +327,6 @@ |
| 534 | compare(viewShowMessageSpy.signalArguments[2][0], "You should have seen three messages"); |
| 535 | } |
| 536 | |
| 537 | - function test_waiting() { |
| 538 | - // Make sure we unset 'waiting' on prompt |
| 539 | - selectUser("has-password"); |
| 540 | - compare(greeter.waiting, true); |
| 541 | - tryCompare(greeter, "waiting", false); |
| 542 | - |
| 543 | - // Make sure we unset 'waiting' on authentication |
| 544 | - selectUser("no-password"); |
| 545 | - compare(greeter.waiting, true); |
| 546 | - tryCompare(greeter, "waiting", false); |
| 547 | - } |
| 548 | - |
| 549 | function test_locked() { |
| 550 | selectUser("has-password"); |
| 551 | compare(view.locked, true); |
| 552 | |
| 553 | === modified file 'tests/qmltests/Greeter/tst_WideView.qml' |
| 554 | --- tests/qmltests/Greeter/tst_WideView.qml 2016-07-06 18:42:19 +0000 |
| 555 | +++ tests/qmltests/Greeter/tst_WideView.qml 2016-07-06 18:42:21 +0000 |
| 556 | @@ -63,7 +63,8 @@ |
| 557 | } |
| 558 | |
| 559 | onSelected: { |
| 560 | - currentIndexField.text = index; |
| 561 | + if (index >= 0) |
| 562 | + currentIndexField.text = index; |
| 563 | } |
| 564 | |
| 565 | QtObject { |
| 566 | @@ -392,7 +393,7 @@ |
| 567 | |
| 568 | function test_respondedWithPassword() { |
| 569 | view.locked = true; |
| 570 | - view.showPrompt("Prompt", true, true); |
| 571 | + view.showPrompt("Prompt", true, false); |
| 572 | var passwordInput = findChild(view, "passwordInput"); |
| 573 | compare(passwordInput.text, "Prompt"); |
| 574 | verify(passwordInput.isSecret); |
| 575 | @@ -454,13 +455,13 @@ |
| 576 | view.showPrompt("Prompt", true, true); |
| 577 | var promptField = findChild(view, "promptField"); |
| 578 | tap(promptField); |
| 579 | - compare(promptField.focus, true); |
| 580 | - compare(promptField.enabled, true); |
| 581 | + verify(promptField.activeFocus); |
| 582 | + compare(promptField.opacity, 1); |
| 583 | |
| 584 | typeString("password"); |
| 585 | keyClick(Qt.Key_Enter); |
| 586 | - compare(promptField.focus, true); |
| 587 | - compare(promptField.enabled, false); |
| 588 | + verify(promptField.activeFocus); |
| 589 | + compare(promptField.opacity, 0); // hidden by fakeLabel |
| 590 | |
| 591 | compare(selectedSpy.count, 0); |
| 592 | keyClick(Qt.Key_Escape); |
| 593 | @@ -468,8 +469,8 @@ |
| 594 | compare(selectedSpy.signalArguments[0][0], 1); |
| 595 | |
| 596 | view.reset(); |
| 597 | - compare(promptField.focus, false); |
| 598 | - compare(promptField.enabled, true); |
| 599 | + verify(promptField.activeFocus); |
| 600 | + compare(promptField.opacity, 1); |
| 601 | } |
| 602 | |
| 603 | function test_unicode() { |
| 604 | @@ -489,6 +490,7 @@ |
| 605 | compare(selectedSpy.signalArguments[0][0], 0); |
| 606 | selectedSpy.clear(); |
| 607 | |
| 608 | + view.reset(); |
| 609 | view.locked = false; |
| 610 | compare(passwordInput.text, "Log In"); |
| 611 | tap(passwordInput); |
| 612 | @@ -518,13 +520,89 @@ |
| 613 | tryCompare(loginList, "height", view.height); |
| 614 | } |
| 615 | |
| 616 | - function test_alphanumeric() { |
| 617 | - var passwordInput = findChild(view, "passwordInput"); |
| 618 | + function test_passphrase() { |
| 619 | + var promptField = findChild(view, "promptField"); |
| 620 | + view.showPrompt("", true, true); |
| 621 | |
| 622 | verify(view.alphanumeric); |
| 623 | - verify(passwordInput.isAlphanumeric); |
| 624 | + compare(promptField.inputMethodHints & Qt.ImhDigitsOnly, 0); |
| 625 | + |
| 626 | + keyClick(Qt.Key_D); |
| 627 | + compare(promptField.text, "d"); |
| 628 | + } |
| 629 | + |
| 630 | + function test_passcode() { |
| 631 | + var promptField = findChild(view, "promptField"); |
| 632 | + view.showPrompt("", true, true); |
| 633 | + |
| 634 | view.alphanumeric = false; |
| 635 | - verify(!passwordInput.isAlphanumeric); |
| 636 | + compare(promptField.inputMethodHints & Qt.ImhDigitsOnly, Qt.ImhDigitsOnly); |
| 637 | + |
| 638 | + keyClick(Qt.Key_D); |
| 639 | + compare(promptField.text, ""); |
| 640 | + |
| 641 | + keyClick(Qt.Key_0); |
| 642 | + keyClick(Qt.Key_0); |
| 643 | + keyClick(Qt.Key_0); |
| 644 | + keyClick(Qt.Key_0); |
| 645 | + compare(promptField.text, "0000"); |
| 646 | + |
| 647 | + compare(respondedSpy.count, 1); |
| 648 | + compare(respondedSpy.signalArguments[0][0], "0000"); |
| 649 | + |
| 650 | + compare(promptField.opacity, 0); |
| 651 | + } |
| 652 | + |
| 653 | + function test_loginListMovement_data() { |
| 654 | + return [ |
| 655 | + {tag: "up", key: Qt.Key_Up, result: -1}, |
| 656 | + {tag: "down", key: Qt.Key_Down, result: 1}, |
| 657 | + ] |
| 658 | + } |
| 659 | + |
| 660 | + function test_loginListMovement(data) { |
| 661 | + keyClick(data.key); |
| 662 | + compare(selectedSpy.count, 1); |
| 663 | + compare(selectedSpy.signalArguments[0][0], data.result); |
| 664 | + } |
| 665 | + |
| 666 | + function test_focusStaysActive() { |
| 667 | + var promptField = findChild(view, "promptField"); |
| 668 | + var promptButton = findChild(view, "promptButton"); |
| 669 | + |
| 670 | + verify(promptButton.activeFocus); |
| 671 | + keyClick(Qt.Key_Enter); |
| 672 | + compare(selectedSpy.count, 0); |
| 673 | + compare(respondedSpy.count, 1); |
| 674 | + compare(respondedSpy.signalArguments[0][0], ""); |
| 675 | + verify(promptButton.activeFocus); |
| 676 | + keyClick(Qt.Key_Enter); |
| 677 | + compare(respondedSpy.count, 1); |
| 678 | + |
| 679 | + view.showPrompt("", true, true); |
| 680 | + verify(promptField.activeFocus); |
| 681 | + keyClick(Qt.Key_D); |
| 682 | + keyClick(Qt.Key_Enter); |
| 683 | + compare(selectedSpy.count, 0); |
| 684 | + compare(respondedSpy.count, 2); |
| 685 | + compare(respondedSpy.signalArguments[1][0], "d"); |
| 686 | + verify(promptField.activeFocus); |
| 687 | + keyClick(Qt.Key_Enter); |
| 688 | + compare(respondedSpy.count, 2); |
| 689 | + |
| 690 | + view.reset(); |
| 691 | + view.locked = true; |
| 692 | + verify(promptButton.activeFocus); |
| 693 | + keyClick(Qt.Key_Enter); |
| 694 | + compare(respondedSpy.count, 2); |
| 695 | + compare(selectedSpy.count, 1); |
| 696 | + compare(selectedSpy.signalArguments[0][0], 0); |
| 697 | + verify(promptButton.activeFocus); |
| 698 | + keyClick(Qt.Key_Enter); |
| 699 | + compare(selectedSpy.count, 1); |
| 700 | + |
| 701 | + view.showPrompt("", true, true); |
| 702 | + verify(promptField.activeFocus); |
| 703 | } |
| 704 | } |
| 705 | } |
| 706 | |
| 707 | === modified file 'tests/qmltests/Tutorial/tst_Tutorial.qml' |
| 708 | --- tests/qmltests/Tutorial/tst_Tutorial.qml 2016-07-06 18:42:19 +0000 |
| 709 | +++ tests/qmltests/Tutorial/tst_Tutorial.qml 2016-07-06 18:42:21 +0000 |
| 710 | @@ -296,7 +296,7 @@ |
| 711 | } |
| 712 | |
| 713 | function prepareShell() { |
| 714 | - tryCompare(shell, "enabled", true); // enabled by greeter when ready |
| 715 | + tryCompare(shell, "waitingOnGreeter", false); // reset by greeter when ready |
| 716 | |
| 717 | WindowStateStorage.clear(); |
| 718 | SurfaceManager.inputMethodSurface.setState(Mir.MinimizedState); |
| 719 | |
| 720 | === modified file 'tests/qmltests/tst_OrientedShell.qml' |
| 721 | --- tests/qmltests/tst_OrientedShell.qml 2016-07-06 18:42:19 +0000 |
| 722 | +++ tests/qmltests/tst_OrientedShell.qml 2016-07-06 18:42:21 +0000 |
| 723 | @@ -493,7 +493,7 @@ |
| 724 | } |
| 725 | |
| 726 | function cleanup() { |
| 727 | - tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state |
| 728 | + tryCompare(shell, "waitingOnGreeter", false); // make sure greeter didn't leave us in disabled state |
| 729 | shell = null; |
| 730 | topLevelSurfaceList = null; |
| 731 | |
| 732 | @@ -1583,7 +1583,7 @@ |
| 733 | var stageLoader = findChild(shell, "applicationsDisplayLoader"); |
| 734 | verify(stageLoader); |
| 735 | |
| 736 | - tryCompare(shell, "enabled", true); // enabled by greeter when ready |
| 737 | + tryCompare(shell, "waitingOnGreeter", false); // reset by greeter when ready |
| 738 | |
| 739 | waitUntilShellIsInOrientation(root.physicalOrientation0); |
| 740 | |
| 741 | |
| 742 | === modified file 'tests/qmltests/tst_Shell.qml' |
| 743 | --- tests/qmltests/tst_Shell.qml 2016-07-06 18:42:19 +0000 |
| 744 | +++ tests/qmltests/tst_Shell.qml 2016-07-06 18:42:21 +0000 |
| 745 | @@ -456,7 +456,7 @@ |
| 746 | function cleanup() { |
| 747 | waitForRendering(shell); |
| 748 | mouseEmulation.checked = true; |
| 749 | - tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state |
| 750 | + tryCompare(shell, "waitingOnGreeter", false); // make sure greeter didn't leave us in disabled state |
| 751 | tearDown(); |
| 752 | WindowStateStorage.clear(); |
| 753 | } |
| 754 | @@ -466,7 +466,7 @@ |
| 755 | shellLoader.active = true; |
| 756 | tryCompare(shellLoader, "status", Loader.Ready); |
| 757 | removeTimeConstraintsFromSwipeAreas(shellLoader.item); |
| 758 | - tryCompare(shell, "enabled", true); // enabled by greeter when ready |
| 759 | + tryCompare(shell, "waitingOnGreeter", false); // reset by greeter when ready |
| 760 | |
| 761 | sessionSpy.target = findChild(shell, "greeter") |
| 762 | dashCommunicatorSpy.target = findInvisibleChild(shell, "dashCommunicator"); |
| 763 | @@ -818,7 +818,7 @@ |
| 764 | tryCompare(userlist, "currentIndex", next) |
| 765 | tryCompare(userlist, "movingInternally", false) |
| 766 | } |
| 767 | - tryCompare(shell, "enabled", true); // wait for PAM to settle |
| 768 | + tryCompare(shell, "waitingOnGreeter", false); // wait for PAM to settle |
| 769 | } |
| 770 | |
| 771 | function selectUser(name) { |
| 772 | |
| 773 | === modified file 'tests/qmltests/tst_ShellWithPin.qml' |
| 774 | --- tests/qmltests/tst_ShellWithPin.qml 2016-07-06 18:42:19 +0000 |
| 775 | +++ tests/qmltests/tst_ShellWithPin.qml 2016-07-06 18:42:21 +0000 |
| 776 | @@ -150,7 +150,7 @@ |
| 777 | property Item shell: shellLoader.status === Loader.Ready ? shellLoader.item : null |
| 778 | |
| 779 | function init() { |
| 780 | - tryCompare(shell, "enabled", true); // will be enabled when greeter is all ready |
| 781 | + tryCompare(shell, "waitingOnGreeter", false); // will be set when greeter is all ready |
| 782 | var greeter = findChild(shell, "greeter"); |
| 783 | sessionSpy.target = greeter; |
| 784 | swipeAwayGreeter(true); |
| 785 | @@ -165,7 +165,7 @@ |
| 786 | } |
| 787 | |
| 788 | function cleanup() { |
| 789 | - tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state |
| 790 | + tryCompare(shell, "waitingOnGreeter", false); // make sure greeter didn't leave us in disabled state |
| 791 | |
| 792 | shellLoader.itemDestroyed = false |
| 793 | |
| 794 | @@ -533,10 +533,12 @@ |
| 795 | |
| 796 | // Confirm that we start disabled |
| 797 | compare(promptSpy.count, 0); |
| 798 | - verify(!shell.enabled); |
| 799 | + verify(shell.waitingOnGreeter); |
| 800 | + var coverPageDragHandle = findChild(shell, "coverPageDragHandle"); |
| 801 | + verify(!coverPageDragHandle.enabled); |
| 802 | |
| 803 | // And that we only become enabled once the lockscreen is up |
| 804 | - tryCompare(shell, "enabled", true); |
| 805 | + tryCompare(shell, "waitingOnGreeter", false); |
| 806 | verify(promptSpy.count > 0); |
| 807 | var lockscreen = findChild(shell, "lockscreen"); |
| 808 | verify(lockscreen.shown); |
I'm going to work on some qmltests now, but this branch can be reviewed and tested in meantime.