Kiwix-android: When using kiwix search from selected text, no button works in the search activity.

Created on 11 Mar 2020  路  20Comments  路  Source: kiwix/kiwix-android

Describe the bug

When selecting text from a zim file, a sub menu appears. By clicking on 'Search Kiwix' in this submenu the search activity is started. This opened search activity is impossible to exit without restarting the application entirely.

Expected behavior

I don't actually know what the 'Search Kiwix' sub-menu button should do. Perhaps it should open the search activity with the selected text inserted into the search field? Or maybe it should be completely removed.

Steps to reproduce the behavior:

  1. Open a zim file.
  2. Select arbritary text.
  3. Choose the 'Search Kiwix' option from the expandable '...'.
  4. Try exiting this view by using any of the back buttons or the search button.

Screenshots

https://imgur.com/a/JAMamPO

Environment

  • Version of Kiwix Android : 3.2.1 Build 6230201
  • Device : Tested on OnePlus 5 (Physical) and Pixel 2 (Virtual)
  • OS version : 9.0.10 OxygenOS for OnePlus 5 and Android 10.0 for the Pixel 2

Logs

https://pastebin.com/XziRrPmh

bug

Most helpful comment

A strong contender for the right solution.

All 20 comments

Is this fixed? Noticed issue #1681 is similar.

No, that was the app shortcut I believe. I don't really now what the expected behaviour is.

it should open the search activity with the selected text inserted into the search field

I'd say go for this

Ok. I'll try fixing this.

I think this error is coming from the weird pattern of restarting the MainActivity to launch SearchActivity, if you have "Don't keep Activities" turned on this will trap you in SearchActivity as MainActivity only exists to send you to SearchActivity

Ok, so starting the search activity from the current activity might solve the issue?

A strong contender for the right solution.

@macgills What I have noticed this far is that in onResume in CoreMainActivity this happens:

      Intent intent = getIntent();
      if (intent.getAction() != null) {
        switch (intent.getAction()) {
          case Intent.ACTION_PROCESS_TEXT: {
            ...
            Intent i = new Intent(this, SearchActivity.class);
            startActivityForResult(i, MainMenuKt.REQUEST_FILE_SEARCH);
            break;
          }
    ...

As the SearchActivity returns its result, onResume is called again for CoreMainActivity, However, as the same intent that started the SearchActivity is still the intent that is returned by getIntent(), CoreMainActivity starts the SearchaActivity again. To test this, I used setIntent(null) before starting the SearchActivity, which indeed solves the issue but is poor style.

Intent i = new Intent(this, SearchActivity.class);
setIntent(null);
startActivityForResult(i, MainMenuKt.REQUEST_FILE_SEARCH);

I can't figure out where the text-selection menu item Search Kiwix is coming from. If I have the Test Custom App installed, it is added as an option to the text-selection menu as well (as Search Test Custom App). The better solution would be to start the activity from the click listener on the menu item Search Kiwix but I can't find it. Any ideas?

Have a look at OpenFile, instead of this startForResult stuff we can just start the activity with new data and it will either go to onCreate or onNewIntent

Ok, it seems to be working now! The following is what adds the 'Search Kiwix' option to the text selector: in core/AndroidManifest.xml

<intent-filter android:label="@string/app_search_string">
  <action android:name="android.intent.action.PROCESS_TEXT" />
  <category android:name="android.intent.category.DEFAULT" />
  <data android:mimeType="text/plain" />
</intent-filter>

As this tutorial recommends [https://medium.com/androiddevelopers/custom-text-selection-actions-with-action-process-text-191f792d2999], intents should not be checked in onResume but in onCreate and in onNewIntent so I extracted the switch cases from onResume and onNewIntent to a method and called it from onCreate instead of onResume. Which prevents the infinite loop.

The current behavior is that the selected text is searched for in the SearchActivity, and the results are displayed. Which seems like the correct behavior.

Try also long pressing the kiwix app icon and selecting "Search" from there.

Also try all of this with a combination of "Don't keep activities" on and off. I am not sure if these 2 features reuse logic portions or not

@macgills
The search widget seems to be working with "Don't keep activities" off but not on. The same goes for the text selection menu.

Then get opening that PR, woo!

Hehe, no sorry we can't celebrate yet! I meant it did not work with "Don't Keep activites". It works with "keep activites" i.e. "Don't keep activites" off.

馃槩 misread. Well see if they use the same codepath. If they don't then you don't have to worry about "Search Shortcut" as this ticket is about "Search Text"

馃槩 Seems to use the same code path for when the activity is started but not when sending an intent to onNewIntent.. However, the text selection menu also stops working when "Don't keep activities" is on. Will investigate further!

The problem here is that it starts MainActivity with an intent to start another activity, when MainActivity gets recreated it just sends you straight back to Search.

In older applications I used an "IntentHandlingActivity" that was totally transparent and had noHistory set so you couldn't navigate back once it forwarded you on to an Activity.

There is also much more complex things that are achievable if you read the very impenetrable documentation here

Ok, I think now is the time to celebrate. A least a little. I fixed so that the selection search menu-item works with and without "Don't keep activities". However, the search widget only works some times (with and without "Don't keep activities" ). But as you said, that is another issue that I will post.

@macgills Oh missed your comment. My solution was to check

if (savedInstanceState == null) {
      handleIntentActions(getIntent());
}

As savedInstanceState is null only on inital onCreate.

@macgills What is the intended behavior of the search widget if the app is closed? Should it open the search activity without a zim file or just open the app?

As it is now. The search widget opens the app without going to the search activity if the app is currently not open. If the app is open in the background the search widget opens the search activity with the currently opened book/zim file.

That behaviour seems correct @Frans-Lukas

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kelson42 picture kelson42  路  5Comments

chstdu picture chstdu  路  6Comments

Frans-Lukas picture Frans-Lukas  路  4Comments

brijeshshah13 picture brijeshshah13  路  4Comments

brijeshshah13 picture brijeshshah13  路  4Comments