Image extension fails to notify of source changes

Bug #1401920 reported by Olivier Tilloy
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
Critical
Zoltan Balogh
ubuntu-ui-toolkit (Ubuntu RTM)
Fix Released
Critical
Cris Dywan

Bug Description

When importing Ubuntu.Components, the base Image QML type is enhanced (with the UCQQuickImageExtension class) to add transparent support for scaling assets.

This breaks the emission of the sourceChanged signal. Application developers cannot implement the onSourceChanged slot to be notified of changes to the value of the 'source' property, and this isn’t documented anywhere.

Another issue I’m observing with this custom extension is that an image is not reloaded if I do the following, and invoke the reload() function on the image (this works if I comment out the import of Ubuntu.Components):

    Image {
        cache: false
        function reload() {
            var s = source
            source = ""
            source = s
        }
    }

Similarly (and maybe even more worrying), actually changing the value of the source property (by appending a fragment to the URL) doesn’t trigger a reload either, as demonstrated by the attached example (requires a random image named "test.jpg" in the same folder).

Related branches

Revision history for this message
Olivier Tilloy (osomon) wrote :
description: updated
Revision history for this message
Olivier Tilloy (osomon) wrote :

The bug still exists with the latest vivid-proposed as of today.

Revision history for this message
Zsombor Egri (zsombi) wrote :

I checked the issue with .svg and .jpg images, they both seem to work. sourceChanged() signal is emitted and the source is changed accordingly.

I have tested with QtQuick 2.4 and UITK 1.3.

Changed in ubuntu-ui-toolkit (Ubuntu):
status: New → Incomplete
Revision history for this message
Olivier Tilloy (osomon) wrote :

Please test again. Just open the attached QML file in qmlscene (ensure that there is a test.jpg image in the same directory), and verify that nothing is being printed out to the console. If you comment out the import of Ubuntu.Components, you get logs for source changed and status changed every second.

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Incomplete → New
Revision history for this message
Zsombor Egri (zsombi) wrote :

Indeed, the sourtceChanged() comes when the toolkit import is removed. However the Image is not unloaded, even though the source doesn't exist.

Revision history for this message
Zsombor Egri (zsombi) wrote :

Ok, so the source you gave is a pretty invalid one, the QQuickImageBase - without the toolkit extension - reports that the source had changed, and changes the url, but the actual image is never changed/unloaded, because they also do QQmlFile::urlToLocalFileOrQrc(url), which will cut the #1450427565454 from the end.

In our case, we call this method way before we pass the given url to the QQuickImageBase, therefore the source change does not happen.

Changed in ubuntu-ui-toolkit (Ubuntu):
assignee: nobody → Zsombor Egri (zsombi)
importance: Undecided → Critical
status: New → In Progress
Revision history for this message
Olivier Tilloy (osomon) wrote :

The source with a fragment is not invalid: since it’s a URL, when converting to a local file name the fragment should be removed, but that doesn’t make the URL invalid.
This fragment trick is typically used to ensure an image is reloaded without changing the actual file path.

Zoltan Balogh (bzoltan)
affects: ubuntu-ui-toolkit (Ubuntu) → ubuntu-ui-toolkit (Ubuntu RTM)
Cris Dywan (kalikiana)
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
assignee: Zsombor Egri (zsombi) → Christian Dywan (kalikiana)
Revision history for this message
Łukasz Zemczak (sil2100) wrote :
Download full text (3.4 KiB)

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

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

ubuntu-ui-toolkit (1.3.1795+15.04.20160106-0ubuntu1) vivid; urgency=medium

  [ Christian Dywan ]
  * Fix typo in runtest.sh: $? is the error code, not $*.
  * Document use of RegExpValidator for password/ PIN input.
  * Include filename derived QML class name in .api. Fixes LP: #1527138
  * Use edit-clear for clear button instead of the invalid clear search.
  * No transparency in focus outline.
  * Give the original Toolbar its name back.
  * Fix typo in runtest.sh: $? is the error code, not $*.
  * Use resolved filename but add fragment. Fixes LP: #1401920

  [ Zoltán Balogh ]
  * Use build root for the Gestures library instead of relative path.
  * Fix the tests for the gallery change

  [ Zsombor Egri ]
  * Add mnemonics support to Action. Fixes LP: #1527527
  * Use original Image.source in image extension when no scaling is needed in
    order to keep the formatting which would be otherwise removed from a
    resolved file URL. Fixes LP: #1401920
  * Move UbuntuListView 1.3 code into one single file. Fixes LP: #1523815

  [ Albert Astals Cid ]
  * Add UCTestExtras::removeTimeConstraintsFromSwipeArea
    For that needed to move ucswipearea.cpp from UbuntuComponents to
    UbuntuGestures but made it in a private header so we don't have to care much
    for the API/ABI stability for now.

  [ Andrea Bernabei ]
  * make scrollbars tests more robust.

  [ Tim Peeters ]
  * Update font sizes. Fixes LP: #1528263

  [ CI Train Bot ]
  * No-change rebuild.

ubuntu-ui-toolkit (1.3.1778+16.04.20151221-0ubuntu1) xenial; urgency=medium

  [ Tim Peeters ]
  * Fix issue with trigger() for ListItems.Standard. (LP: #1527811)

ubuntu-ui-toolkit (1.3.1778+16.04.20151217.1-0ubuntu1) xenial; urgency=medium

  [ Andrea Bernabei ]
  * Rework Scrollbars and add ScrollView component.

  [ Zsombor Egri ]
  * Refactor UCSwipeAreaPrivate to use QQuickItemPrivate instead of being
    a QObject.

  [ Tim Peeters ]
  * Reduce toolbar height on phone in landscape orientation.
    Fixes LP: #1336793.
  * Introduce the new Toolbar component for the header edit mode.
  * Reduce the PageHeader height by 1 GU on phone in landscape orientation.
    Fixes LP: #1336793
  * Fix AppHeader 1.2 to use the new MathUtils. This fixes a failing autopilot
    CPO test.

  [ Zoltan Balogh ]
  * Do not wipe when provisioning for normal use and improve
    regression detection.
  * Try to link Gestures more explicitely. (kudos to xnox).

  [ Christian Dywan ]
  * Proper keyboard navigation and focus visuals for buttons.
    Fixes LP: #1225139, LP: #1514850
  * Escape should close popover. Fixes LP: #1523828
  * Surround state overloading message with static boolean. Fixes LP: #1523399
  * Don't exit, return 1 from export_modules_dir.sh.

  [ Michael Sheldon ]
  * Set the activeFocus on the input component when a text input area receive
    focus. Fixes LP: #1518352

  [ Benjamin Zeller ]
  * Move PageTreeNode to C++.
  * Custom property flag needs to be set always even when the value itself does not chan...

Read more...

Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: In Progress → Fix Released
Zoltan Balogh (bzoltan)
no longer affects: ubuntu-ui-toolkit (Ubuntu)
Changed in canonical-devices-system-image:
status: New → Fix Committed
importance: Undecided → Critical
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  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

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