Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug]: Duplicate search option crashes firefox in android #23009

Closed
doctor82 opened this issue Dec 29, 2021 · 24 comments
Closed

[Bug]: Duplicate search option crashes firefox in android #23009

doctor82 opened this issue Dec 29, 2021 · 24 comments
Assignees
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, .. Feature:ContextMenu Menu that appears when long-pressing on website content Feature:Search needs:gv GeckoView bug required to fix the issue. See bugzilla.mozilla.org qa-triaged Issues triaged by qa

Comments

@doctor82
Copy link

doctor82 commented Dec 29, 2021

Steps to reproduce

In my Firefox Android, I have two search buttons popping up on long pressing any word. This happens even when no add-ons are installed. The first one works fine and the second one crashes the browser. Tried reinstalling the browser many times with no use.

Expected behaviour

Should go to google search

Actual behaviour

Browser crashes

Device name

Vivo u20

Android version

Android 9

Firefox release type

Firefox

Firefox version

95.2.0

Device logs

https://crash-stats.mozilla.org/report/index/75573ecb-4180-4e92-aa5b-318c00211223

Additional information

_No response
firefox
_

┆Issue is synchronized with this Jira Task

@doctor82 doctor82 added needs:triage Issue needs triage 🐞 bug Crashes, Something isn't working, .. labels Dec 29, 2021
@kbrosnan
Copy link
Contributor

android.util.AndroidRuntimeException: Calling startActivity() from outside of an Activity  context requires the FLAG_ACTIVITY_NEW_TASK flag. Is this really what you want?
	at android.app.ContextImpl.startActivity(ContextImpl.java:931)
	at android.app.ContextImpl.startActivity(ContextImpl.java:907)
	at android.content.ContextWrapper.startActivity(ContextWrapper.java:379)
	at android.content.ContextWrapper.startActivity(ContextWrapper.java:379)
	at com.android.internal.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:159)
	at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:923)
	at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:913)
	at com.android.internal.view.FloatingActionMode.lambda$setFloatingToolbar$0(FloatingActionMode.java:129)
	at com.android.internal.view.-$$Lambda$FloatingActionMode$LU5MpPuKYDtwlFAuYhXYfzgLNLE.onMenuItemClick(Unknown Source:2)
	at com.android.internal.widget.FloatingToolbar$FloatingToolbarPopup$2.onClick(FloatingToolbar.java:509)
	at android.view.View.performClick(View.java:6643)
	at android.view.View.performClickInternal(View.java:6620)
	at android.view.View.access$3100(View.java:790)
	at android.view.View$PerformClick.run(View.java:26217)
	at android.os.Handler.handleCallback(Handler.java:873)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:224)
	at android.app.ActivityThread.main(ActivityThread.java:7134)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:876)

@kbrosnan kbrosnan added b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info Feature:ContextMenu Menu that appears when long-pressing on website content Feature:Search and removed needs:triage Issue needs triage labels Dec 29, 2021
@Amejia481 Amejia481 added the eng:qa:needed QA Needed label Dec 29, 2021
@Amejia481
Copy link
Contributor

I was trying to simulate without luck, I tried with Pixel 3 with Android 11 and Moto G5 with Android 7 and an emulator with Android 9. QA team could you help us to identify in which devices you are able to replicated, apart from Vivo u20.

@doctor82
Copy link
Author

I'm not a techie and cannot understand the above conversation. Maybe a factory reset can solve the problem. But to reinstall and set up the device from scratch seems a pain compared to just avoiding the duplicate search button.

@SoftVision-LorandJanos
Copy link

SoftVision-LorandJanos commented Dec 30, 2021

I couldn't reproduce this issue either.
No second "Search" button appears.
I have two "Search privately" buttons, they were set automatically, and none of them produces a crash.
Tested on the following devices:

  • Xiaomi Mi11 Lite (Android 11).
  • Huawei P30 (Android 10).
  • Samsung Galaxy Tab S3 (Android 9).
  • Motorola Nexus 6 (Android 7.1.1).
  • LG Nexus 5 (Android 6.0.1).

Tested on the following builds:

  • latest Nightly 97.0a1 (2021-12-29).
  • latest Beta (96.0.0-beta5).
  • latest Release (95.2.0).

@doctor82 Could you try this on the latest Nightly, see if you reproduce there also? Thanks!

@SoftVision-LorandJanos SoftVision-LorandJanos added qa-triaged Issues triaged by qa and removed eng:qa:needed QA Needed labels Dec 30, 2021
@kbrosnan
Copy link
Contributor

This is going to depend on what is adding the second search option on the context menu. Maybe we should go to an allowlist for constructing the context menu?

@Mugurell
Copy link
Contributor

Mugurell commented Dec 30, 2021

@doctor82 This issue probably depends on what other apps you have installed.
When you are selecting some text in the app Fenix offers some default options (like Copy/Search) and also asks other installed apps if they can provide some useful actions for the user based on that text.
In this case I guess another installed app says that it can help to Search with that selected text.
To find out what this app is you can use

adb shell pm dump * | grep 'android.intent.action.PROCESS_TEXT' -A 1

(If you don't have adb installed you can follow the steps from here - https://www.xda-developers.com/install-adb-windows-macos-linux/ to get it working)

@Mugurell
Copy link
Contributor

Mugurell commented Dec 30, 2021

The logcat is interesting though because I think it shows us crashing when we try to open that other app and this really shouldn't happen so the crash could be a device specific issue.

@doctor82
Copy link
Author

I am not a techie so not able to give more inputs. I believe it could have caused by some other applications previously installed.
I did a factory reset and reinstalled firefox and problem vanished. I believe that firefox should not have given other applications a privilege to install context menu items. Not planning to install any unnecessary apps again. Thank you all for your valuable time and support.

@kbrosnan kbrosnan reopened this Dec 31, 2021
@kbrosnan
Copy link
Contributor

Please don't close this issue. It is a valid crash and we may have other ways to get the info we need.

@ouyen
Copy link

ouyen commented Jun 17, 2022

I also encountered this problem. After run adb command, get the result below:

$ adb shell pm dump * | grep 'android.intent.action.PROCESS_TEXT' -A 1
      android.intent.action.PROCESS_TEXT:
        a1e24f9 com.microsoft.launcher/com.microsoft.bing.ProcessTextSearch

So I uninstall the microsoft launcher, and the problem is solved.

My phone is Redmi K20 pro, use MIUI by xiaomi.eu 12.5.6. (Android 11)

@lolp1
Copy link

lolp1 commented Jul 27, 2022

I can confirm I have the exact same error, and the exact same solution. I should note even if I change my default launcher away from Microsoft launcher, I still get the error. I must uninstall it completely in order to resolve the error.

@Mugurell
Copy link
Contributor

Thank you for updating this!
I can also easily reproduce with having the Microsoft launcher app installed.
And it seems like we can prevent the crash and the extraneous "Search" custom action appearing in the floating menu.
Opened https://bugzilla.mozilla.org/show_bug.cgi?id=1781780 for this.

@Mugurell Mugurell added the needs:gv GeckoView bug required to fix the issue. See bugzilla.mozilla.org label Jul 27, 2022
@doctor82
Copy link
Author

I can confirm that I had Microsoft launcher installed at some point of time, prior to factory resetting my phone.

@lolp1
Copy link

lolp1 commented Jul 27, 2022

Thank you for updating this! I can also easily reproduce with having the Microsoft launcher app installed. And it seems like we can prevent the crash and the extraneous "Search" custom action appearing in the floating menu. Opened https://bugzilla.mozilla.org/show_bug.cgi?id=1781780 for this.

I am a software engineer by trade, but am new to Kotlin and mobile app work. I downloaded the repo hear and followed the steps to get started and to debug the project upon getting this error and joined the matrix chat and wound up here. Just to be sure, the issue is that this should not show up in the floating menu as an action, even though it should not, due to the export flag being set to false but it does anyways due to a lack of a check, am I reading and understanding that correctly?

@Mugurell
Copy link
Contributor

Mugurell commented Jul 28, 2022

@lolp1 Yes, we want to exclude private activities from being added as custom actions since they cannot be started from outside.

@lolp1
Copy link

lolp1 commented Jul 28, 2022

@lolp1 Yes, we want to exclude private activities from being added as custom actions since they cannot be started from outside.

@Mugurell

So I am wanting to fix this my self on a local build for now, which is why I am asking. I am still unsure how you can extract if the activity is private or not, it seems like there is an export field in the manifest data but google seems to show no obvious ways to resolve that information.

@Mugurell
Copy link
Contributor

Mugurell commented Jul 28, 2022

So I am wanting to fix this my self on a local build for now, which is why I am asking. I am still unsure how you can extract if the activity is private or not, it seems like there is an export field in the manifest data but google seems to show no obvious ways to resolve that information.

This issue is to be resolved in Geckoview (see https://bugzilla.mozilla.org/show_bug.cgi?id=1781780) not in Fenix/Android-Components.
To check whether the activity is exported or not we can use something like

<PackageManager>.queryIntentActivities(<processTextIntent>, PackageManager.MATCH_DEFAULT_ONLY)[0].activityInfo.exported

@lolp1 If you want to patch a local build you could combine the Android-Components code with the GeckoView code with what I wrote above about excluding private activities and not call super anymore.

@lolp1
Copy link

lolp1 commented Jul 28, 2022

@Mugurell

EDIT: It works! Thanks for the info. I actually wanted to get my feet wet with this stuff, but needed a little motivation, and fixing this error was it because it drove me nuts. Will await some one to submit a proper ideal solution and update it when its pushed.

For reference:

ActivityInfo info =<Intent>.resolveActivityInfo(<Context>.getPackageManager(),
                             PackageManager.MATCH_DEFAULT_ONLY);
boolean show = info != null && info.exported;
if(show) {
    return true;
}

@Mugurell
Copy link
Contributor

Mugurell commented Jul 28, 2022

@lolp1 I recommend you wait for an official fix but if you want to thinker the code you posted above used in the combined Android-Components + GeckoView code in place of these lines would probably work.
Note that pm.queryIntentActivities returns a list of all activities matching the provided intent, you would probably have to iterate through all elements and with this approach it seems like you'll not be able to exclude microsoft but keep others, it's all or nothing.

@lolp1
Copy link

lolp1 commented Jul 28, 2022

@Mugurell
I just added a check explicitly for it to be Microsoft's, since the info is available through ActivityInfo.

ActivityInfo info =<Intent>.resolveActivityInfo(<Context>.getPackageManager(),
                             PackageManager.MATCH_DEFAULT_ONLY);

Inside there is various info about the activity, including several unique identifiers to microsoft launcher.

@Mugurell
Copy link
Contributor

@lolp1 Yes but with just those modifications I think in the scenario of having two components responding to that intent - one that we want to exclude and one that we don't, if we are returning false then we won't be keeping the second one - no custom action for android.intent.action.PROCESS_TEXT will be added.
This is to be accounted for with the official fix.

@lolp1
Copy link

lolp1 commented Jul 28, 2022

@Mugurell

I guess the code I pasted is confusing. I simply skip the adding of any one that is not exported. I don't actually use the name heh. There is no good reason to add to the menu an action not exported -- as I found out thanks to every one in this issue thread.

So where it's adding the custom actions, the isActionAvailable will be false if export is also false

@lolp1
Copy link

lolp1 commented Sep 10, 2022

Was this fixed? I'm no longer crashing upon search actions.

EDIT: I was accidentally using my local version with the modification that gets rid of this bug, thinking I was using the official version, sorry, it's still broke.

@zmckenney zmckenney self-assigned this Dec 21, 2022
@cpeterso
Copy link

I'm closing this GitHub issue because we're now tracking this bug in Bugzilla here: https://bugzilla.mozilla.org/show_bug.cgi?id=1781780

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, .. Feature:ContextMenu Menu that appears when long-pressing on website content Feature:Search needs:gv GeckoView bug required to fix the issue. See bugzilla.mozilla.org qa-triaged Issues triaged by qa
Projects
None yet
Development

No branches or pull requests

9 participants