today scope breaks if searching and then clearing the search term

Bug #1572154 reported by Michael Zanetti on 2016-04-19
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
High
Alejandro J. Cura
NearBy Scope
High
Penk Chen
News Scope
High
Penk Chen
Photos Scope
High
Penk Chen
Today Scope
High
Penk Chen
scope-aggregator
High
Kyle Nitzsche

Bug Description

Steps to reproduce:

* go to the today scope
* click the search icon
* search for some random word, e.g. "bla"
* wait for the search to complete (it might already behave a little funny at this point)
* clear the search word, then click the cancel button in the header

=> after some seconds of refreshing, most, if not all of the content of the scope will be missing.

Related branches

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Hi Michael,

Confirmed. note these exceptions.

[2016-04-19 10:25:34.436] ERROR: com.canonical.scopes.dashboard_dashboard: ReplyObject::push(): ReplyObject::push(VariantMap): unity::InvalidArgumentException: CategorisedResult::set_category(): Category must not be null
[2016-04-19 10:25:34.467] ERROR: com.canonical.scopes.dashboard_dashboard: ReplyObject::push(): ReplyObject::push(VariantMap): unity::InvalidArgumentException: CategorisedResult::set_category(): Category must not be null
[2016-04-19 10:25:34.477] ERROR: com.canonical.scopes.dashboard_dashboard: ReplyObject::finished(): unity::InvalidArgumentException: CategorisedResult::set_category(): Category must not be null

Changed in today-scope:
status: New → Confirmed
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

This is a scope aggregator bug, not a today scope bug.

I find the bug was introduced between scope-aggregator bzr tags 1.5.29 and 4.0 (4.0 was the next tag after 1.5.29).

Changed in scope-aggregator:
status: New → Confirmed
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Specifically, the exception starts appearing in bzr rev 150 of scope aggregator.

Changed in canonical-devices-system-image:
assignee: nobody → Alejandro J. Cura (alecu)
importance: Undecided → High
status: New → Confirmed
Changed in scope-aggregator:
importance: Undecided → High
assignee: nobody → Kyle Nitzsche (knitzsche)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

I find that on tapping Cancel after entering a search query, the scope's run() method executes twice. This seems to be not hanlded well by scope-aggregator.

For example: calls-scope always prints "CALLS starting" on entering run():
While watching scope-registry.log:
(launch or refresh calls-scope)
CAllS starting
(enter query string)
CALLS starting
(tap Cancel)
CALLS starting
CALLS starting

There should not be two entries after tapping cancel.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

comment #4 is legitimate and I discussed it with unity-scopes-api member who is looking into it. but it is not the cause of this bug.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

The cause of the bug is registering a category the first time it is needed, saving its ptr to a var, and then reusing it as needed.

But, sometimes, the category pointer no longer exists, for example when a user taps Cancel after a search.

The solution has two parts:

1) monitor whether the query is still valid and return as quickly as possibly if the query is not valid. This is the case when the user taps Cancel: the query is not valid any more. This is general good housekeeping that notices when the user has tapped Cancel. Per comment #4, in this use case the shell issues two queries in rapid succession. The agg scope needs to notice the first is cancelled and start fresh with the second.

2) don't use a var to hold a pointer to a category. Instead, every time a category is needed: check with the unity scope shell to see if the category is registered and if it is not, the register it. Then get the pointer to the category from the shell, and use it directly.

In general, the code for point 2) looks like this:
                         if (!upstream_reply->lookup_category(cat_id))
                            {
                               upstream_reply->register_category
                               (
                                    cat_id,
                                    cat_title,
                                    "",
                                    us::CategoryRenderer(rdr)
                               );
                             }
                             res.set_category(upstream_reply->lookup_category(cat_id));

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

After the merge is reviewed and done, we will need to rebase the agg scopes on scope-aggregator 4.7 .so file. So I am adding three agg scope projects.

Changed in nearby-scope:
status: New → Confirmed
Changed in news-scope:
status: New → Confirmed
Changed in photos-scope:
status: New → Confirmed
Changed in nearby-scope:
importance: Undecided → High
Changed in news-scope:
importance: Undecided → High
Changed in photos-scope:
importance: Undecided → High
Changed in today-scope:
importance: Undecided → High
Changed in nearby-scope:
assignee: nobody → Kyle Nitzsche (knitzsche)
Changed in news-scope:
assignee: nobody → Kyle Nitzsche (knitzsche)
Changed in photos-scope:
assignee: nobody → Kyle Nitzsche (knitzsche)
Changed in today-scope:
assignee: nobody → Kyle Nitzsche (knitzsche)
tags: added: ota11
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

This bug is fixed in scope-aggregator via the merge at the top resulting in scope-aggregator 4.7.

Now I will pull its .so into the four agg scopes with bug tasks, fixing them, for QA and release as versions 4.7.0

Changed in scope-aggregator:
status: Confirmed → Fix Released
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Penk, please find today scope (com.canonical.scopes.dashboard_4.7.0_armhf.click) attached for QA and release in ota11.

Changed in today-scope:
status: Confirmed → Fix Committed
assignee: Kyle Nitzsche (knitzsche) → Penk Chen (penk)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Penk, please find news scope (com.canonical.scopes.news_4.7.0_armhf.click) attached for QA and release in ota11. Thanks!

Changed in news-scope:
status: Confirmed → Fix Committed
assignee: Kyle Nitzsche (knitzsche) → Penk Chen (penk)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Penk, please find photos scope (com.canonical.scopes.photos_4.7.0_armhf.click) attached for QA and release in OTA11. thanks!

Changed in photos-scope:
status: Confirmed → Fix Committed
assignee: Kyle Nitzsche (knitzsche) → Penk Chen (penk)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Penk, please find nearby-scope (com.canonical.unity-scope-nearby_4.7.0_armhf.click) attached for QA and release in OTA11. Thanks!

Changed in nearby-scope:
status: Confirmed → Fix Committed
assignee: Kyle Nitzsche (knitzsche) → Penk Chen (penk)
Penk Chen (penk) on 2016-04-29
tags: added: ota12
removed: ota11
Changed in canonical-devices-system-image:
status: Confirmed → In Progress
milestone: none → 11
Revision history for this message
Pat McGowan (pat-mcgowan) wrote :

@penk Friday you said you would make new tarballs containing these to try to land in 11 right?

Revision history for this message
Penk Chen (penk) wrote : Re: [Bug 1572154] Re: today scope breaks if searching and then clearing the search term

Hi Pat,

It's national holiday here in Taiwan on Monday, and whether I can build new
tarball or not depends on the image importer, it has to be copied over
rc-proposed/*-proposed channels before I have new artifacts in Jenkins.

Best,
penk

On Mon, May 2, 2016 at 11:56 PM, Pat McGowan <email address hidden>
wrote:

> @penk Friday you said you would make new tarballs containing these to
> try to land in 11 right?
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1572154
>
> Title:
> today scope breaks if searching and then clearing the search term
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/canonical-devices-system-image/+bug/1572154/+subscriptions
>

Penk Chen (penk) on 2016-05-03
Changed in nearby-scope:
assignee: Penk Chen (penk) → Elsa Wang (elsawang)
Changed in news-scope:
assignee: Penk Chen (penk) → Elsa Wang (elsawang)
Changed in photos-scope:
assignee: Penk Chen (penk) → Elsa Wang (elsawang)
Changed in today-scope:
assignee: Penk Chen (penk) → Elsa Wang (elsawang)
Elsa Wang (elsawang) on 2016-05-03
Changed in nearby-scope:
assignee: Elsa Wang (elsawang) → Ethan Chang (ethan.chang)
Changed in news-scope:
assignee: Elsa Wang (elsawang) → Ethan Chang (ethan.chang)
Changed in photos-scope:
assignee: Elsa Wang (elsawang) → Ethan Chang (ethan.chang)
Changed in today-scope:
assignee: Elsa Wang (elsawang) → Ethan Chang (ethan.chang)
Revision history for this message
Ethan Chang (ethan.chang) wrote :

Verified those packages on Arale.
I checked that scope still can show contents correctly after cancel the search.

tags: added: cqa-verified
Changed in nearby-scope:
assignee: Ethan Chang (ethan.chang) → Penk Chen (penk)
Changed in news-scope:
assignee: Ethan Chang (ethan.chang) → Penk Chen (penk)
Changed in photos-scope:
assignee: Ethan Chang (ethan.chang) → Penk Chen (penk)
Changed in today-scope:
assignee: Ethan Chang (ethan.chang) → Penk Chen (penk)
Revision history for this message
Ethan Chang (ethan.chang) wrote :

@Kyle
Nearby is not list as favorite scope after re-install 4.7.0 version.
Please help to check.

Changed in nearby-scope:
assignee: Penk Chen (penk) → Kyle Nitzsche (knitzsche)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Hi Ethan,

I confirm the issue with nearby dropping from favorites.

The issue is related to the fact that nearby scope historically did not use a fully qualified name (pkg_app). But now it does.

With historical nearby scope:
phablet@ubuntu-phablet:~$ gsettings get com.canonical.Unity.Dash favorite-scopes
['scope://clickscope', 'scope://com.canonical.scopes.dashboard_dashboard', 'scope://unity-scope-nearby']

With 4..7.0 nearby scope:
phablet@ubuntu-phablet:~$ gsettings get com.canonical.Unity.Dash favorite-scopes
['scope://clickscope', 'scope://com.canonical.scopes.dashboard_dashboard', 'scope://com.canonical.unity-scope-nearby_unity-scope-nearby']

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

The bug was introduced here when autopilot was implemented: http://bazaar.launchpad.net/~nearby-team/nearby-scope/trunk/revision/78.

Line 71 shows that previously the scope.ini file was copied to the historical APP name:
cp $SOURCE/aggregator/scope.ini $TARGET/$APP/${APP}.ini

With this revision the scope ini file was copied to the fully qualified PKG_APP name, which breaks continuity with favorites:
cp $SOURCE/aggregator/scope.ini $TARGET/$APP/${PKG}_${APP}.ini

I will release 4.7.1 version of nearby with that change reverted and attach here.

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

adding 4.7.1 nearby as per comment #17 and #18.

Changed in nearby-scope:
assignee: Kyle Nitzsche (knitzsche) → Ethan Chang (ethan.chang)
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Please use 4.7.2 attached here. 4.7.2 also reverts the file name of the .so to match the ini file.

Revision history for this message
Ethan Chang (ethan.chang) wrote :

Verified the nearby with 4.7.2, the issue fixed

Changed in canonical-devices-system-image:
status: In Progress → Fix Committed
Changed in canonical-devices-system-image:
status: Fix Committed → Fix Released
Revision history for this message
Jin (jindallo) wrote :

Since the feature is released,
update the status.

Changed in news-scope:
status: Fix Committed → Fix Released
Changed in photos-scope:
status: Fix Committed → Fix Released
Changed in today-scope:
status: Fix Committed → Fix Released
Changed in nearby-scope:
assignee: Ethan Chang (ethan.chang) → Penk Chen (penk)
Jin (jindallo) on 2016-08-12
Changed in nearby-scope:
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