ActionBarSherlock icon indicating copy to clipboard operation
ActionBarSherlock copied to clipboard

Pressed action item not dispatched until exiting Activity

Open yukuku opened this issue 12 years ago • 22 comments

This worked in 4.0.0, but no longer work on 4.0.2. After trying each commit to find out where it fails, I found that it worked at 6337076 but failed to work at fca84af "Use the mule to determine proper fragment menu dispatch results."

I have an Activity (subclass of SherlockFragmentActivity) containing a fragment (subclass of SherlockFragment). The fragment calls setHasOptionsMenu(true) and inflates a menu item at onCreateOptionsMenu. Hence the (only) action item button is shown on the right corner of the action bar.

Somehow, when I click on the action item, the pressed state is not displayed. It is as though the action item button is not pressed at all. However, when I click on another button (not actionbar button) at the same activity that opens another activity, suddenly the onOptionsItemSelected of the fragment is called!

This happens in 2.3.7, tried in both Nexus One CM 9.2.0-RC1 and Intel x86 emulator. This works correctly in ICS (4.0.2).

Thanks for writing such a good library!

yukuku avatar Apr 18 '12 19:04 yukuku

I would love a small demo app or if you could condense it into a single class and paste it somewhere.

JakeWharton avatar Apr 18 '12 19:04 JakeWharton

I tried to strip most of the other classes etc, and the bug disappeared :( Currently it's very hard to isolate it. I spend an hour to make the activity as similar as the project that exhibit the bug, but it failed.. Unfortunately the project is closed source, but I would like to help as much as I can. The commit in ABS that broke it was fca84af only. Should I try to locate lines within that commit that broke it? Will the apk of the app help finding the problem? (probably with DEBUG set to true in the ABS classes)

yukuku avatar Apr 18 '12 21:04 yukuku

That commit was introduced to workaround a complication in the support library where it always returns true for menu creation which was leading to problems with invalidation. The new behavior more closely emulates what happens natively so I'm surprised to hear about this regression.

Make sure you are returning true and false appropriately in onCreateOptionsMenu and onOptionsItemSelected. I'll do my best to try and reproduce this.

And if you want to send an APK with DEBUG = true in both ActionBarSherlock.java and SherlockFragmentActivity.java that would be fantastic.

JakeWharton avatar Apr 18 '12 21:04 JakeWharton

Great. The onCreateOptionsMenu I have where the return value is a boolean is on the activity. It's as below:

@Override public boolean onCreateOptionsMenu(Menu menu) {
    if (menu != null) getSupportMenuInflater().inflate(R.menu.activity_main, menu);
    return true;
}

As for the other onCreateOptionsMenus on the fragments, the return values are void.

The onOptionsItemSelected on the fragment is as follows:

@Override public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
    case R.id.menuAbout: {
        SoundEffect.play(Sfx.plastic);
        startActivity(AboutActivity.createIntent());
    } return true;
    default:
        return false;
    }
}

I tried to change the default case to return super.onOptionsItemSelected(item) but it seems that the super implementation also just return false, so it's the same.

No implementation of onOptionsItemSelected is written on the activity itself. Only the fragment.

The apk with DEBUG set to true on both the classes you mentioned is downloadable here: http://dl.dropbox.com/u/128693/tmp-absbug/Turbanizer.apk

The bug happens on the first screen after the splash screen where there is (i) button on the top-right corner. To reproduce the bug: tap the (i) button, observe that nothing happens, then tap the select from album button. Suddenly the About screen appears, which should have happened when the (i) button is pressed.

yukuku avatar Apr 19 '12 19:04 yukuku

@yukuku what is 2.3.7? I couldn't recreate this issue using 2.3.3...

levinotik avatar Apr 29 '12 04:04 levinotik

According to http://en.wikipedia.org/wiki/Android_version_history#Gingerbread_2.3.x, 2.3.7 is for GWallet Support in Nexus S 4G

@yukuku, why return true is after { } for R.id.menuAbout?

iNoles avatar Apr 29 '12 06:04 iNoles

Thanks @iNoles. Guess its not a release I can create an AVD for, huh?

@yukuku Wanted onOptionsItemSelected to return true. He could have done without the brackets altogether, but either way that's a legitimate return from the case statement because he hasn't ended the statement with ; yet. Remember you can sprinkle {} wherever you want in java.

levinotik avatar Apr 29 '12 11:04 levinotik

@levinotik Yes, the purpose of the { } in the case statement is to create a new scope, in order that each case on the switch statement more self-contained. It doesn't affect the flow.

@iNoles Version 2.3.7 is used by CyanogenMod, and I have tried this on the Intel x86 Emulator (you can create the AVD for this, it's the API Level 10 Intel Atom x86 System Image -- which seem to be Android 2.3.7, too (from system Settings -> About phone), although in the download list it shows as 2.3.3.

yukuku avatar May 03 '12 10:05 yukuku

I'm seeing this issue as well. Building against Java 1.6 using ABS 4.0.2.

I've hacked up a test project that is currently exhibiting the issue in a 2.3.3 emulator and 2.3.4 G2 phone. http://dl.dropbox.com/u/5246704/absOptionMenuTest.zip It's ugly because I just stripped out everything I could while still being able to reproduce the issue.

I've also tried the fixes listed in issue 272 with no luck. This is not unexpected as 272 was brought up before the 4.0.2 release so I'm assuming that 4.0.2 includes the fixes mentioned in that issue. Issue 272 has a great description of the problem though.

This issue only appeared in my project after upgrading from ABS 3.5.1.

Let me know if I can provide any other information.

jmcaffee avatar May 07 '12 17:05 jmcaffee

I managed to isolate the bug and I can share the complete project tree for examination.

Source download: http://dl.dropbox.com/u/128693/tmp-absbug/turbanizerabsbug-1.zip

  • the project is located in angelsgate/TurbanizerAbsFail directory.

APK download (for convenience): http://dl.dropbox.com/u/128693/tmp-absbug/turbanizerabsbug-1.apk

ABS version 4.1.0.


Apparently it happens only if the splash screen is shown.

In order to show the bug, when running the app on Android 2.3.x, tap on the About (i) button on top-right, then nothing happens. Then tap on the "Select from album" button on the bottom. Suddenly both the "Whatever" and "About" activity comes out.

Strangely, if we start the MainActivity without showing the splash screen, everything works well. You can try it by running MainActivity with boolean extra showSplash = false. e.g. by running

adb shell am start -n yuku.turbanizerabsfail/.ac.MainActivity --ez showSplash false

yukuku avatar May 21 '12 06:05 yukuku

I really think Splash screen is wrong for Android.

If you really want to do Splash screen then you should do Splash > MainActivity instead Main > Splash > back to Main

iNoles avatar May 21 '12 07:05 iNoles

@iNoles I agree with you. However this is a client's order which I have argued not to put splash screen into and I lost.

Btw, it's not that different from having another activity on top of Main and then back from that activity to Main.

yukuku avatar May 21 '12 07:05 yukuku

This issue bugs me as well. The action bar button doesn't dispatch pressed state until any other UI event is happened, for instance, pressing a button (hw or sw), scrolling a list, etc. Can reproduce this in both 4.0.1 and 4.1.0

pechlambda avatar May 23 '12 12:05 pechlambda

Is there some temporary workaround for the issue?

StenaviN avatar Jun 04 '12 20:06 StenaviN

My issue was due to a race condition between ViewPager and ActionBar. If you're using both of these, you might want to look at Issue #476.

I've also written a post about how I solved the problem if you're interested: http://www.ktechsystems.com/actionbar-menu-items-firing-delayed-events/

jmcaffee avatar Jun 05 '12 18:06 jmcaffee

Hmm… I'll take a look onto your workaround, but I have just an activity which displays three buttons and FrameLayout placeholder for three different fragments. Each of fragments has own menu, inflated from xml. And in some conditions I get this issue.

StenaviN avatar Jun 06 '12 21:06 StenaviN

I have the same issue. When clicking on an action item, onOptionsItemSelected is not called before I press another hardware button or use the homeAsUp button (HTC Desire). Seeing this also only on Android 2.3.7 (CM 7) with ABS 4.1. ICS and JB both work fine with the same code. Can I expect this to be fixed in the near future or should I rather implement the workaround suggested by jmcaffee?

Edit: By reading through some other issues, I came across problems when calling invalidateOptionsMenu(). When I don´t call this method - which I don´t need on Android 2 devices - then I can click the action items normally.

Konsumierer avatar Aug 20 '12 09:08 Konsumierer

I had the same symptoms. There may be multiple ways to cause this problem, but here's one (and a workaround).

Setup:

  • ABS 4.2.0
  • Android 2.2.1 (4.x works correctly)
  • A split action bar and dynamic menu items depending on the tab
  • A derivative of FragmentTabsPager modified to invalidate the options menu when switching tabs (needed to repopulate the menu when sliding rather than explicitly tapping the tab).

Problem: When all menu items are added from the fragments and no menu items are added in the main activity, the action item presses are queued up until switching the tab. This only happens in portrait mode - landscape seems to work fine.

Workaround: Adding a menu item in the parent activity with the SHOW_AS_ACTION_NEVER option ensures that the message processing occurs, but no additional ActionBar actions are shown. It might also work to remove the placeholder menu item in each of the fragments but I didn't try it that way.

Pseudo-analysis: It seems that in my setup the message pump for the lower action bar is only started if the parent activity adds menu items. There may be a cleaner way to work around that, but at least this gives a starting point to investigate.

Hopefully I included enough detail to make this clear (and hopefully this behavior isn't due to other bugs in my code).

TimMackenzie avatar Oct 31 '12 19:10 TimMackenzie

Ok, I solve this problem. In Activity, onOptionsItemSelected return false. very simple.

Yria avatar Nov 08 '12 19:11 Yria

I am still seeing the issue here in android 2.2.2 and using ActionBarSherlock 4.3.1. Is work still ongoing or should I just never upgrade from ActionBarSherlock 4.0.0?

foens avatar Jul 16 '13 08:07 foens

Seeing the issue is marked as "Needs-More-Info", I have created a test application that shows the error.

It is located here: https://github.com/foens/abserrortest With prebuilt apk: https://github.com/foens/abserrortest/raw/master/out/production/abstest/abstest.apk

It contains an activity with a fragment inside. The fragment shows one menu item, but initially its visibility is set to false. Then supportInvalidateOptionsMenu is called and it is set to visible.

Now, pressing the menu item and rotating the device makes the application crash. If the item were initially visible (change line 15 of TestFragment to true), it works as expected.

Hope this helps bashing this bug as it really annoys me.

foens avatar Jul 16 '13 13:07 foens

I had the very same issue on Android 2.2.2. The menu had been drawn correctly, but it wouldn't react on click events until the Fragment had been switched at least once. The Message containing the "click event" was only dispatched after the Fragment disappeared. After this, the onClick() and the dispatchX methods worked as expected only stopping at the isAdded/isVisible test in the Watson.class

I finally solved it by using the proposed "patch" in https://github.com/jakewharton/actionbarsherlock/issues/272 (exchanging ICSLinearlayout with a standard LinearLayout) So in a wild guess, i would think that ICSLinearLayout is blocking/spamming the message queue somehow.

ghost avatar Jul 17 '13 10:07 ghost