Ubuntu
ubuntu-ui-toolkit package

units.gu() emits changed signal before completion

Bug #1647415 reported by Michael Zanetti
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
ubuntu-ui-toolkit (Ubuntu)
Fix Released
Undecided
Cris Dywan

Bug Description

Consider this snippet of code:

-----------------------------------------------
import QtQuick 2.4
import Ubuntu.Components 1.3

Item {
    property int bla: units.gu(10)
    onBlaChanged: print("bla changed", bla);

    property int blubb: 80
    onBlubbChanged: print("blubb changed", blubb);

    Component.onCompleted: print("completed", bla);
}
----------------------------------------------

The output of this is:

qml: bla changed 80
qml: completed 80

the changed signal should not be emitted before completion, given it is its initial value and not actually changed.

The current behavior triggers all the onChanged handlers when there's units.gu() involved, which results in things like change animations being played at startup and lots of wasted cpu cycles for evaluating all those changed events.

See original description
Tags: unity8

Related branches

lp://staging/~ubuntu-sdk-team/ubuntu-ui-toolkit/outTheWindow
ubuntu-sdk-build-bot: Approve (continuous-integration)
Zsombor Egri: Approve
Loïc Molinari (community): Approve
Diff: 790 lines (+606/-11)
15 files modified
src/UbuntuToolkit/UbuntuToolkit.pro (+3/-0)
src/UbuntuToolkit/ubuntutoolkitmodule.cpp (+2/-0)
src/UbuntuToolkit/ucmainviewbase.cpp (+1/-0)
src/UbuntuToolkit/ucmainwindow.cpp (+215/-0)
src/UbuntuToolkit/ucmainwindow_p.h (+73/-0)
src/UbuntuToolkit/ucmainwindow_p_p.h (+47/-0)
src/UbuntuToolkit/ucunits.cpp (+32/-6)
src/UbuntuToolkit/ucunits_p.h (+5/-0)
tests/autopilot/ubuntuuitoolkit/tests/test_launcher.mainwindow.qml (+12/-5)
tests/autopilot/ubuntuuitoolkit/tests/test_launcher.py (+12/-0)
tests/autopilot/ubuntuuitoolkit/tests/test_launcher.window.qml (+43/-0)
tests/unit/mainwindow/AppName.qml (+28/-0)
tests/unit/mainwindow/mainwindow.pro (+5/-0)
tests/unit/mainwindow/tst_mainwindow.cpp (+127/-0)
tests/unit/unit.pro (+1/-0)
lp://staging/~ci-train-bot/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntu-zesty-2418 (Merged)
Michael Zanetti (mzanetti)
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in ubuntu-ui-toolkit (Ubuntu):
status: New → Confirmed
Gerry Boland (gerboland)
tags: added: unity8
Cris Dywan (kalikiana)
Changed in ubuntu-ui-toolkit (Ubuntu):
assignee: nobody → Christian Dywan (kalikiana)
status: Confirmed → In Progress
Revision history for this message
Cris Dywan (kalikiana) wrote :

Just to add my thoughts here for the record (as we discussed it in IRC as well); since this is a context property that depends effectively on a/the Window, I think evaluation goes like this: 1. bindings are evaluated 2. context property is set as the a/window becomes available 3. completion happens - as such it may not be fixable by design of context properties in general. But bug 1587431 aka MainWindow is introducing a units property on the window itself, and as such should be solving this problem if you can use that component instead of your regular QQuickWindow (and basically all apps should eventually).

Revision history for this message
Cris Dywan (kalikiana) wrote :

The MainWindow went in, so I will consider this sorted, provided one is using it - by design, as I said, I don't see a way to make it work with context properties. Do let me know, though, if you still see any bugs with the regular property.

Changed in ubuntu-ui-toolkit (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-ui-toolkit - 1.3.2166+17.04.20170130

---------------
ubuntu-ui-toolkit (1.3.2166+17.04.20170130) zesty; urgency=medium

  [ Andrea Bernabei ]
  * ListItemLayout documentation: fix typo in color usage of a code example.
    Fixes LP: #1561861

  [ Zsombor Egri ]
  * Get tst_listitem_focus_13 back alive. Fixes LP: #1624331.

  [ Arthur Mello ]
  * Add support for interacting with Content Hub Clipboard UI via DBus calls.
    Fixes LP: #1563440.

  [ Daniel d'Andrada ]
  * If GRID_UNIT_PX set, always use it. It should override scaling information
    from the QPA.

  [ Adnane Belmadiaf ]
  * Expose append method in TextArea. Fixes LP: #1658121.

  [ Tim Peeters ]
  * Toolbar visual refresh to scroll the icons instead of using an overflow
    panel. Fixes LP: #1558018.
  * Add missing dependency for the UITK gallery. Fixes lLP: #1640135

  [ Sergio Cazzolato ]
  * This change is done to remove the dependencies with upstart in the tests.
    To start apps with env vars is gonna be used the launch_test_application
    method provided by autopilot in its test base class which is a wrapper for
    the ubuntu-app-launch tools (API). This change removes all the dependencies
    with upstart, so all the projects which are using the initctl api and
    fixtures will be impacted. An update in the documentation or in the blog is
    gonna be done also to communicate this change to the developers.

  [ Christian Dywan ]
  * Add visualRoot property to MainWindow. Fixes LP: #1504551, LP: #1656857.
  * Add python3-debian to control.gles and harden package-sorting.sh.
  * Add organizationName property to MainWindow
  * Add MainWindow to Ubuntu.Components.Labs.
    Fixes LP: #1573118, LP: #1587431, LP: #1647415

  [ Albert Astals Cid ]
  * Labs is at 1.0.
  * You can't save the focus after the popup/dialog has been created, at that
    stage the dialog has focus already, you need to do it before creating it.

 -- Zoltán Balogh <email address hidden> Mon, 30 Jan 2017 11:30:03 +0000

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.