today scope breaks if searching and then clearing the search term

[Michael Zanetti]

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.

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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).

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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.

[Kyle Nitzsche (knitzsche)]

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.

[Kyle Nitzsche (knitzsche)]

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));

[Kyle Nitzsche (knitzsche)]

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.

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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

[Pat McGowan (pat-mcgowan)]

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

[Penk Chen (penk)]

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
>

[Ethan Chang (ethan.chang)]

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

[Ethan Chang (ethan.chang)]

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

[Kyle Nitzsche (knitzsche)]

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']

[Kyle Nitzsche (knitzsche)]

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.

[Kyle Nitzsche (knitzsche)]

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

[Kyle Nitzsche (knitzsche)]

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

[Ethan Chang (ethan.chang)]

Verified the nearby with 4.7.2, the issue fixed

[Jin (jindallo)]

Since the feature is released,
update the status.