TextField regression/behavior change in 1.3

Bug #1513897 reported by Albert Astals Cid on 2015-11-06
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
High
Zoltan Balogh
ubuntu-ui-toolkit (Ubuntu RTM)
High
Christian Dywan

Bug Description

Having an AbstractButton as secondaryItem of a TextField used to not change the TextField focus status when pressing the button but with 1.3 it does (i.e. the TextField used to not gain focus when pressing on secondaryItem and now it does)

Code at http://paste.ubuntu.com/13125887/

Is this an intended change or a behaviour regression?

Related branches

Zsombor Egri (zsombi) wrote :

Perhaps the TextField got changed, it should keep the content of the overlay Items unfocusable by touch...

Zsombor Egri (zsombi) wrote :

As workaround, set the activeFocusOnPress for the AbstractButton to false. TextField should set them all automatically.

Zsombor Egri (zsombi) on 2015-11-09
Changed in ubuntu-ui-toolkit (Ubuntu):
importance: Undecided → High
status: New → Confirmed
assignee: nobody → Zsombor Egri (zsombi)
Zsombor Egri (zsombi) on 2015-11-09
Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → New
Zsombor Egri (zsombi) on 2015-11-09
Changed in ubuntu-ui-toolkit (Ubuntu):
status: New → Confirmed
summary: - TextField/AbstractButton regression/behavior change in 1.3
+ TextField regression/behavior change in 1.3
Zsombor Egri (zsombi) wrote :

The same happens if you replace the AbstractButton with MouseArea: the text field suffers focus change.

Zsombor Egri (zsombi) on 2015-11-09
Changed in ubuntu-ui-toolkit (Ubuntu):
assignee: Zsombor Egri (zsombi) → Christian Dywan (kalikiana)
Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → In Progress
description: updated
Andrea Bernabei (faenil) wrote :

I spent some time debugging this, I'll leave my findings here:

Situation in 1.2
TextField has both
"activeFocusOnPress: true" AND "property alias activeFocusOnPress: editor.activeFocusOnPress" (I don't know the reason behind the choice of having a property alias with the same name as the property coming from the C++ StyledItem)
and editor, which is a TextInput, has "activeFocusOnPress: true"

What happens in 1.2 when you tap on the button?
TextField doesn't get the focus, because its activeFocusOnPress is never set to true.
In fact, because of the alias declaration, setting "activeFocusOnPress: true" just enables the property of the editor, and doesn't actually change the property of the TextField (this is my guess after debugging the focus logic).
As a consequence, commenting out "activeFocusOnPress: true" in 1.2 TextField doesn't bring any behaviour change, because TextInput's activeFocusOnPress is true by default.

Situation in the current 1.3 (I'm testing on r1688)
The property alias was removed from TextField (this can be considered a fix). As a consequence,
"activeFocusOnPress: true" has the intended behaviour of setting the property of the TextField to true.

What happens in 1.3 when you tap on the button?
TextField gets the focus, because it has activeFocusOnPress set to true, so due to the way the current focus logic works, when the Button gets a mouse event, its parents get focus (if they requested it).
So even if UCAbstractButton has activeFocusOnPress set to false, its parent, TextField, still has it set to true, so it will get the focus whenever the Button receives a click.

Andrea Bernabei (faenil) wrote :

As a consequence of the explanation I left above, even with the current MR (which disables activeFocusOnPress for primary and secondaryItem), TextField will still get the focus when those items are clicked

Andrea Bernabei (faenil) wrote :

Another observation that I should have added in the long comment:

what I understand from reading the code is that the author of the component probably did not intend 1.2 to work that way. The way 1.2 TextField behaves is just a consequence of an excessive alias which made the initialization of TextField's activeFocusOnPress have another effect instead (i.e. set TextInput's activeFocusOnPress)

Changed in ubuntu-ui-toolkit (Ubuntu):
status: In Progress → Fix Committed
Zoltan Balogh (bzoltan) on 2015-12-05
affects: ubuntu-ui-toolkit (Ubuntu) → ubuntu-ui-toolkit (Ubuntu RTM)
Łukasz Zemczak (sil2100) wrote :
Download full text (6.8 KiB)

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

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

ubuntu-ui-toolkit (1.3.1742+15.04.20151209-0ubuntu1) vivid; urgency=medium

  [ Zoltán Balogh ]
  * Enable building the UITK tests in any case.
  * Remove Q_Q(UCStyledItemBase) because of unused variable warning.
  * Resolve Trusty specific compiler issue not resolving QPointer data on
    connections.
  * Use only the object name when selecting sections and labels in tests.

  [Zsombor Egri ]
  * Update BottomEdge with UI/UX agreed during the sprint. Fixes LP: #1368811.
  * BottomEdge component. Fixes LP: #1368811.
  * Remove build failure on Xenial caused by deprecated Qt functions.
  * Fix BottomEdgeHint consumes activeFocusOnPress handling. Fixes LP: #1517777
  * Migrate DirectionalDragArea from Unity8, named as SwipeArea. Original code
    (from lp:unity8) by: Daniel d'Andrada <email address hidden>.
  * Delete QQuickView when the application quits. Fixing segfault caused by the
    dangling objects in GestireDetector.
  * BottomEdgeHint API changes, deprecating state property, introducing locked
    property to drive visuals lock and click handling.
  * Fixing CheckBox and Switch getting checked property altered after clicked()
    signal is emitted. Fixes LP: #1510919.
  * Add ListItem.swipeEnabled property to block swiping when overlay MouseArea
    is used to drag content. Fixes swiping when leading/trailing actions list
    is empty. Fixes LP: #1500409, LP: #1500416.
  * Follow the new design in BottomEdgeHint.
  * Provide import version information for StyledItem and remove theme.version
    obsolete property.
  * Use QQuickItemChangeListener to listen parent changes, skipping the meta
    object model. Convert UCThemingExtension into a Q_INTERFACE so object_cast<>
    can work with it saving the need to memorise on an item whether it is an
    extended item or not.
  * Set mouseAttached to false to revert regression in UITK gallery.
  * Remove BottomEdgeHint from UITK gallery's MainPage.qml which occludes with
    the bottom-up dragging of the ListView.

  [ Benjamin Zeller ]
  * Avoid unnecessary updates for i18n strings.
  * "MathUtils.clamp, min value should not be bigger than the max value".
    Fixes LP: #1520557.
  * Move MathUtils to Cpp.

  [ Loïc Molinari ]
  * UbuntuShape - Fixed deprecation logging issues.
  * This fix prevents logging a deprecation warning for "image", "color" and
    "gradientColor" properties when the import version is less than 1.3. The
    logging of properties used internally (through the old image wrapper) have
    been removed too since the user might not even have used them.
    Fixes LP: #1519414.

  [ Christian Dywan ]
  * Enable gallery target in qmake.
  * Don't set activeFocusOnPress on TextField but on child only.
    Fixes LP: #1486274, LP: #1513897.
  * Make runtest.sh work out of the box again.
  * Explicitly handle keyboard anchoring in dialog foreground.
    Fixes LP: #1376763.
  * Remove GestureDetector in favor of SwipeArea in BottomEdgeHint. Also make
    possible to assign Action to the ...

Read more...

Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: Fix Committed → Fix Released
Changed in canonical-devices-system-image:
status: New → Fix Committed
importance: Undecided → High
assignee: nobody → Zoltan Balogh (bzoltan)
milestone: none → ww02-2016
Changed in canonical-devices-system-image:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers