SystemSettings Language&Text view: hitting Space on HW keyboard triggers switch even when it (or its list item) does not show any visual focus frame

Bug #1549733 reported by Andrea Bernabei on 2016-02-25
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
High
Zoltan Balogh
ubuntu-system-settings (Ubuntu)
Undecided
Unassigned
ubuntu-ui-toolkit (Ubuntu)
High
Unassigned
ubuntu-ui-toolkit (Ubuntu RTM)
High
Unassigned

Bug Description

Nexus7, rc-proposed, r373
Ubuntu UI Toolkit version r1795

1) Connect bluetooth keyboard
2) Open system settings
3) Open Language & Text view
4) Press tab until the n-th switch shows its focus frame
5) Now tap (using touchscreen) on the m-th list item, with m < n (tap on the list item, not on the switch of that list item)
NOTE: It is important that m < n, the bug will not trigger on listitems that have not been focused at least once!
6) At this point the focus frame around the n-th switch has disappeared
7) Now press Space on the keyboard

Expected outcome:
Nothing, because there is no focus frame anywhere on the screen
This is confusing for the user as he can never be sure about which item will be actioned by the keyboard keys

Actual outcome:
The switch of the m-th list item is triggered, even though that list item or switch were not showing any focus frame

<https://goo.gl/LCxiWO>: “To avoid errors from unintended focus and distraction from irrelevant focus rings, there should be three classes of control: … • Non-pointer-focusable: Focusable with Tab … Yes … Focusable with pointing device … No”

<https://goo.gl/fscyyx>: “To avoid distraction from irrelevant selection, there should be three classes of list: … • Non-pointer-selectable: Selectable with arrow keys … Yes … Selectable with pointing device … No … Any list that is non-pointer-selectable should be non-pointer-focusable.

Related branches

Andrea Bernabei (faenil) on 2016-02-25
summary: - List items focus in SystemSettings Language&Text view misbehaves
+ SystemSettings Language&Text view: hitting Space on HW keyboard triggers
+ switch even when it (or its list item) does not show any visual focus
+ frame
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in ubuntu-system-settings (Ubuntu):
status: New → Confirmed
Changed in ubuntu-ui-toolkit (Ubuntu):
status: New → Confirmed
Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → In Progress
importance: Undecided → High
Zoltan Balogh (bzoltan) on 2016-03-17
Changed in ubuntu-ui-toolkit (Ubuntu):
status: In Progress → Fix Committed
no longer affects: ubuntu-system-settings (Ubuntu RTM)
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: New → Fix Committed
importance: Undecided → High
Changed in canonical-devices-system-image:
assignee: nobody → Zoltan Balogh (bzoltan)
status: New → In Progress
importance: Undecided → High
milestone: none → 11
Łukasz Zemczak (sil2100) wrote :
Download full text (4.1 KiB)

This bug was fixed in the package ubuntu-ui-toolkit 1.3.1918+15.04.20160404-0ubuntu1 in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/stable-phone-overlay

---------------

ubuntu-ui-toolkit (1.3.1918+15.04.20160404-0ubuntu1) vivid; urgency=medium

  [ Benjamin Zeller ]
  * Register expected warning messages in the tree testcase.
  * Add testsuite for Tree. Fixes LP: #1549171.
  * Compile with Qt 5.6. Fixes LP: #1540356
  * More Qt 5.6 fixes.

  [ Michael Terry ]
  * Add grabGesture field to SwipeArea.
  * This field controls whether the SwipeArea takes ownership of the gestures it
    observes. Fixes LP: #1527387.

  [ Timo Jyrinki ]
  * Add a test for blocking if packaging not sorted and clean.
  * More package name transitioning, wrap and sort, fix debian/copyright syntax,
    update standards-version, fix devel package sections and add/fix
    lintian-overrides.

  [ Olivier Tilloy ]
  * Fix dependencies of the transitional dummy package
    qtdeclarative5-ubuntu-ui-toolkit-plugin. Fixes LP: #1553551.

  [ Christian Dywan ]
  * Rename qtdeclarative5-*-plugin to qml-module-*. Fixes LP: #1342031.
  * Require a qtdeclarative5 known to have the tab fence patch.
  * Reset previous theme version and use UbuntuTestCase in tst_animator.
  * BottomEdge(Hint) focus and close via keyboard. Fixes LP: #1523825
  * Add 1.3 version of UbuntuTestCase.
  * No gdb when running unit tests on armv7l.
  * Build-depend on python3-debian for wrap-and-sort.

  [ XiaoGuo Liu ]
  * Fix AdaptivePageLayout documentation.

  [ Tim Peeters ]
  * Properly disable left/right arrows on sections when at the beginning/end of
    the list. Fixes LP: #1551356.
  * Sections and old AppHeader landscape mode. Fixes LP: #1551341.
  * Set the window color as soon as the window is available, to avoid a
    flickering background when it is updated later.
    Fixes LP: #1439133, LP: #1554897
  * Add theme, style and styleName properties to Dialog and Popover.
    Fixes LP: #1555548
  * Update the gallery script to use the new name of the export script.
  * Export the full list of library paths.
  * Prevent Page geometry changes in APL when changing Pages in a column.
  * Fix typo in DatePickerStyle. Fixes: LP #1561440.

  [ Zsombor Egri ]
  * ListItem focus navigation in ListView and other views.
    Fixes LP: #1523815, LP: #1536679, LP: #1549733, LP: #1549743
  * BorromEdge.preloadContent. Fixes LP: #1540454.
  * Asynchronous loader (AsyncLoader) using incubator, prerequisite for
    BottomEdge preloading its content. Fixes LP: #1540454.
  * ListView proxy should not consume up/down key events. Fixes LP: #1554447
  * Pick up the right style. Fixes LP: #1555797
  * Label makes sure the color alteration is known by the theming.
    Fixes LP: #1555784
  * Fix AsyncLoader tests so they create slightly less amount of elements.
  * AdaptivePageLayout removes all pages pushed to next columns when
    addPageToNextColumn() is called. Fixes LP: #1544745
  * Fix BottomEdge content discarding.
  * Disconnect enabled signal in Label in destructor, so functor call will not
    be handled when QQuickItem triggers enabled change. Fixes LP: #1560044.

  [ Andrea Bernabei ]
  * UI G...

Read more...

Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: Fix Committed → Fix Released
Changed in canonical-devices-system-image:
status: In Progress → Fix Committed
Launchpad Janitor (janitor) wrote :
Download full text (4.1 KiB)

This bug was fixed in the package ubuntu-ui-toolkit - 1.3.1918+16.04.20160404-0ubuntu1

---------------
ubuntu-ui-toolkit (1.3.1918+16.04.20160404-0ubuntu1) xenial; urgency=medium

  [ Benjamin Zeller ]
  * Register expected warning messages in the tree testcase.
  * Add testsuite for Tree. Fixes LP: #1549171.
  * Compile with Qt 5.6. Fixes LP: #1540356
  * More Qt 5.6 fixes.

  [ Michael Terry ]
  * Add grabGesture field to SwipeArea.
  * This field controls whether the SwipeArea takes ownership of the gestures it
    observes. Fixes LP: #1527387.

  [ Timo Jyrinki ]
  * Add additional Breaks as requested by archive admin.
  * Add a test for blocking if packaging not sorted and clean.
  * More package name transitioning, wrap and sort, fix debian/copyright syntax,
    update standards-version, fix devel package sections and add/fix
    lintian-overrides.

  [ Olivier Tilloy ]
  * Fix dependencies of the transitional dummy package
    qtdeclarative5-ubuntu-ui-toolkit-plugin. Fixes LP: #1553551.

  [ Christian Dywan ]
  * Rename qtdeclarative5-*-plugin to qml-module-*. Fixes LP: #1342031.
  * Require a qtdeclarative5 known to have the tab fence patch.
  * Reset previous theme version and use UbuntuTestCase in tst_animator.
  * BottomEdge(Hint) focus and close via keyboard. Fixes LP: #1523825
  * Add 1.3 version of UbuntuTestCase.
  * No gdb when running unit tests on armv7l.
  * Build-depend on python3-debian for wrap-and-sort.

  [ XiaoGuo Liu ]
  * Fix AdaptivePageLayout documentation.

  [ Tim Peeters ]
  * Properly disable left/right arrows on sections when at the beginning/end of
    the list. Fixes LP: #1551356.
  * Sections and old AppHeader landscape mode. Fixes LP: #1551341.
  * Set the window color as soon as the window is available, to avoid a
    flickering background when it is updated later.
    Fixes LP: #1439133, LP: #1554897
  * Add theme, style and styleName properties to Dialog and Popover.
    Fixes LP: #1555548
  * Update the gallery script to use the new name of the export script.
  * Export the full list of library paths.
  * Prevent Page geometry changes in APL when changing Pages in a column.
  * Fix typo in DatePickerStyle. Fixes: LP #1561440.

  [ Zsombor Egri ]
  * ListItem focus navigation in ListView and other views.
    Fixes LP: #1523815, LP: #1536679, LP: #1549733, LP: #1549743
  * BorromEdge.preloadContent. Fixes LP: #1540454.
  * Asynchronous loader (AsyncLoader) using incubator, prerequisite for
    BottomEdge preloading its content. Fixes LP: #1540454.
  * ListView proxy should not consume up/down key events. Fixes LP: #1554447
  * Pick up the right style. Fixes LP: #1555797
  * Label makes sure the color alteration is known by the theming.
    Fixes LP: #1555784
  * Fix AsyncLoader tests so they create slightly less amount of elements.
  * AdaptivePageLayout removes all pages pushed to next columns when
    addPageToNextColumn() is called. Fixes LP: #1544745
  * Fix BottomEdge content discarding.
  * Disconnect enabled signal in Label in destructor, so functor call will not
    be handled when QQuickItem triggers enabled change. Fixes LP: #1560044.

  [ Andrea Bernabei ]
  * UI Gallery: fix vertical alig...

Read more...

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Fix Committed → Fix Released
Jonas G. Drange (jonas-drange) wrote :

Should we do something about this in System Settings, or is the fix in UITK also a fix for System Settings?

Changed in ubuntu-system-settings (Ubuntu):
status: Confirmed → Incomplete
Changed in canonical-devices-system-image:
status: Fix Committed → Fix Released
Andrea Bernabei (faenil) wrote :

This is not fixed as of
UITK r1984

Tested on M10 rc-proposed, r119

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Fix Released → Confirmed
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: Fix Released → Confirmed
Changed in canonical-devices-system-image:
status: Fix Released → Confirmed
Changed in canonical-devices-system-image:
milestone: 11 → 12
Christian Dywan (kalikiana) wrote :

There's (currently) no bi-directional connection between keyboard input and the focus ring - the focus ring shows in response to (Shift)Tab and arrows in context of a *ListView. As the most basic example, any (Abstract)Button can be clicked, thus have activeFocus and receive keyboard input as described here, this was true even before the focus ring existed.

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → Incomplete
Andrea Bernabei (faenil) on 2016-06-03
description: updated
Christian Dywan (kalikiana) wrote :

So it seems the problem is that we have no generic design to show "I accept keyboard input". The TextField/Area for instance changes its background color. This is distinct from the focus ring, which on TextField/Area also only shows if (Shift)Tab was used to focus it. Buttons generally don't do that.

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Incomplete → Confirmed
Changed in ubuntu-ux:
assignee: nobody → Matthew Paul Thomas (mpt)
Changed in ubuntu-ux:
status: New → In Progress
Matthew Paul Thomas (mpt) wrote :
Download full text (4.3 KiB)

“For controls, focus refers to which control has first claim on any non-pointing input, such as input from a keyboard…” <https://goo.gl/oZlhEU> (currently Canonical-only link, sorry). That is the only reason for focus to exist, so there should not be any difference between “focused” and “I accept keyboard input”. (Text fields used to, as Christian says, change background color distinct from their focus ring. It’s good that they no longer do, because it made unfocused fields look disabled. So now they should have a focus ring instead … but that’s a separate problem.)

Apart from that, I think any of three changes to the reported behavior would make it unsurprising. But each would have other consequences:

A. Give the switch a focus ring. But this would be too distracting, because — especially on a phone — you hardly ever toggle controls with a hardware keyboard. Furthermore, as “5) … tap on the list item, not on the switch” implies, the switch isn’t (or shouldn’t be) focused anyway. Space is toggling the switch not because it is focused, but because it is the only control inside the selected list item. (Lists of checkbox items should behave the same way: select with arrow keys, Space to toggle.)

B. So, make the selected list item look selected then! But this would be distracting too, because this screen isn’t really a “list” of anything in the non-technical sense; on many other platforms it wouldn’t be using a list control at all. So there’s a larger question about whether System Settings should be using a list control on screens like this. But even in the “Keyboard Layouts” or “Wi-Fi” screens, which do have genuine lists of things, once you tap any of the items it still shouldn’t be visibly selected, because which one you happened to tap on most recently is either not interesting or indicated in some other way. So, this isn’t a System Settings bug.

C. Make tapping not select the list item at all, so Space doesn’t do anything inside the item. Selection inside a list has similarities with focus outside a list (which is one of the reasons people often confuse the two). One similarity is that sometimes it’s appropriate only when you’re actively using an external keyboard. A focus example: If you have a password field focused, you should be able to Tab to the “Show password” checkbox, but for tapping the checkbox to focus it would be annoying, because it would unfocus the password field. So text fields should take focus on tap or Tab, but checkboxes should take focus *only* on Tab. Similarly with list selections: sometimes a list always has exactly one item selected at a time, and in that case selection should always be visible, but in other cases selection is interesting *only* if you’re using a keyboard to navigate through it.

Unfortunately, neither of these subtleties have been specified yet. The focus subtlety should have been part of the recent “focus states” design project, but that project got sidetracked. No-one has yet specified the behavior of lists, as opposed to list items. And the spec for list items uses the word “select” only to refer to activation, not selection.

So, I’ve proposed a change to “General behavior for a...

Read more...

Changed in ubuntu-ux:
status: In Progress → Fix Committed
Changed in ubuntu-system-settings (Ubuntu):
status: Incomplete → Invalid
Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → Triaged
description: updated
Christian Dywan (kalikiana) wrote :

> Text fields used to, as Christian says,
> change background color distinct from their focus ring.
> It’s good that they no longer do,
> because it made unfocused fields look disabled.
> So now they should have a focus ring instead

Text fields show a caret whenever they accept keyboard (or OSK) input - they do show a focus ring only when focused via keyboard.

> A. Give the switch a focus ring

Focus ring on tap would mean changing what focus ring means.

> B. So, make the selected list item look selected then [...]

Selected item? Maybe. How about selected switch?

> C. Make tapping not select the list item [...]

Keyboard focus should follow tap/ click so that pressing (Shift)Tab moves relative to the component last interacted with - that's a requirement right now.
Conceivably the behavior could be changed to 'anything that won't visually show keyboard input takes no focus on tap' provided all components including other types of buttons are re-visited and the keyboard navigation requirement above is re-visited.
This goes back to my point, we have no requirement to show that input is accepted, except for text input as a special-case.

Changed in ubuntu-ux:
status: Fix Committed → In Progress
Christian Dywan (kalikiana) wrote :

Note: I set back the status of the UX task because we still have no solution. Please comment here and update as soon as the spec has been updated and something can be implemented.

Changed in canonical-devices-system-image:
status: Confirmed → Incomplete
milestone: 12 → backlog
description: updated
Matthew Paul Thomas (mpt) wrote :

It is not and has never been a “requirement” that “Keyboard focus should follow tap/ click so that pressing (Shift)Tab moves relative to the component last interacted with”. That would be highly unpleasant if it applied, for example, to scrollbars, panel resize handles, or window resize handles. Unfortunately, because the initial toolkit designers were focused on touch input, the toolkit specification did not make a distinction between elements that should be focusable and elements that should not.

So, Femma has just approved my definition of the three focusability classes for elements, and it is now part of the specification template for toolkit elements to define which focusability class it belongs to. She’s also approved my definition of the three selectability classes for lists. Making lists non-pointer-selectable by default would make *this* list non-pointer-selectable, which should mean tapping the list item will not focus its switch, which should mean that Space does not toggle the switch, which fixes this bug.

To summarize: When most of your input is with touch, it would be ugly for most controls to get focus rings — and most list items to look selected — just because you tapped them and *might* use a keyboard to navigate through them later. The toolkit currently tries to solve this by suppressing the focused/selected appearance based on *how* the item was focused/selected. This has turned out to be mistaken because it results in surprising behavior if you *do* use a keyboard later. A better solution is that for many types of control — and items in many lists — tapping/clicking should not focus/select the control in the first place.

Changed in ubuntu-ux:
status: In Progress → Fix Committed
Changed in canonical-devices-system-image:
status: Incomplete → Confirmed
no longer affects: ubuntu-ux
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers