Skip to content

Fragments added to ChildFragmentManager do not receive calls for OptionsMenus #828

Open
purdyk opened this Issue Feb 11, 2013 · 26 comments
@purdyk
purdyk commented Feb 11, 2013

The latest version of the support library brings in the ChildFragmentManager to all fragments. However, none of the fragments added through childFragmentManager will receive calls to any of the OptionsMenu functions.

Relevant code from the fragment class:

    boolean performCreateOptionsMenu(Menu menu, MenuInflater inflater) {
        boolean show = false;
        if (!mHidden) {
            if (mHasMenu && mMenuVisible) {
                show = true;
                onCreateOptionsMenu(menu, inflater);
            }
            if (mChildFragmentManager != null) {
                show |= mChildFragmentManager.dispatchCreateOptionsMenu(menu, inflater);
            }
        }
        return show;
    }
@JakeWharton
Owner

Just a heads up. I'm not even going to look at this until r11 or newer is in Maven central.

@purdyk
purdyk commented Feb 11, 2013

Resolution for the issue can be found on my fork:
purdyk@30750de
I will issue a pull request if you'd like when you move to r11.

Cheers.

@SimonVT
Collaborator
SimonVT commented Feb 12, 2013

That PR only works for 1 level of nested fragments tho. Our users might have more, you never know.

@purdyk
purdyk commented Feb 12, 2013

Yes, I noticed this earlier today and resolved it in a later commit. Thanks for looking over it.

Properly recursive implementation is here:
purdyk@cab97d6

@SimonVT
Collaborator
SimonVT commented Feb 12, 2013

Protip: Make your commits pass checkstyle checks

@purdyk
purdyk commented Feb 12, 2013

Looks like a handy tool. I've gone over my changes and resolved all the offending lines created by my changes.

@RawPhunky

purdyk I have tried your implementation and found another issue with ViewPager and FragmentPagerAdapter.

FragmentPagerAdapter calls setMenuVisibility... but it does not call it for child fragments.

Lets imagine that our Pager has 3 fragments. And one has a child fragment. All of them have menus.
F1 F2 F3
F1.1

Now let's activate 2nd page with F2.
FragmentPagerAdapter (on setPrimaryItem) will call F1.setMenuVisibility(false), but will not do it for F1.1

So our menu will be F2 + F1.1, not that we expected.

For a temp solution i have modified your code:
I do not dispatch calls to child if parent fragments menu is invisible (you need to do that in all your recursive functions):

private boolean recurseOnPreparePanel(Fragment f, Menu menu)
{
    boolean show = false;
    if (f != null && !f.mHidden && f.mHasMenu && f.mMenuVisible && f instanceof OnPrepareOptionsMenuListener) {
        show = true;
        ((OnPrepareOptionsMenuListener)f).onPrepareOptionsMenu(menu);
    }

    if (!f.mMenuVisible)
        return show;

    // Dispatch calls to any child fragments
    if (f != null && f.mChildFragmentManager != null && f.mChildFragmentManager.mAdded != null)
    {
        for (int j = 0; j < f.mChildFragmentManager.mAdded.size(); j++)
        {
            Fragment f2 = f.mChildFragmentManager.mAdded.get(j);
            show = recurseOnPreparePanel(f2, menu);
        }
    }
    return show;
}
@SimonVT SimonVT closed this Mar 5, 2013
@SimonVT SimonVT reopened this Mar 5, 2013
@SimonVT
Collaborator
SimonVT commented Mar 5, 2013

Whoops.

@SimonVT
Collaborator
SimonVT commented Mar 5, 2013

Make sure any changes your make matches the behavior of the native implementation.

@tkeunebr
tkeunebr commented May 1, 2013

Hey guys! Do you have any idea on when this issue might be addressed?

@Nevro
Nevro commented May 1, 2013

Probably never! All maven library stuck on android 4.1.1.4 and support library r7 forever... :(

@tkeunebr
tkeunebr commented May 2, 2013

Well that's bad news... This issue is actually preventing me from shipping. I modified Watson.java using the above implementation but I'm still getting duplicated MenuItems when I use child fragments along together with a ViewPager.

@Nevro
Nevro commented May 3, 2013

Try with latest abs version and with my watson class. If not work, commit some simple sample to your git and post url here...

@deviant-studio

thanks Nevro, great fix!

@m190
m190 commented May 28, 2013

Hi Nevro,

don't you know why inner fragments menu is not destroyed? :)
I tried to use last abs and your fix, but once created, menu is shown all the time, even if you switch the tab.
I debugged the code (Watson class), and it look like the menu is destroyed, but on the screen I see other things.
Should I override onDestroyOptionsMenu or to do something other?

Thanks.

@tkeunebr

I'm actually having the same issue, would be cool if someone had a fix :)

@Nevro
Nevro commented May 28, 2013

Copy ViewPager from r12 and wait for official ActionBarCompat....

@ekux44
ekux44 commented Jun 22, 2013

I can't believe this issue isn't more public. Took me an entire afternoon to isolate this issue in a codebase I'm backporting with ABS

@JulianCardona

thanks @purdyk your solution did work for me. 👍

@Shusshu
Shusshu commented Jun 27, 2013

Same problem as @m190 with @purdyk or even @Nevro's code
I also changed the support-v4 dependency to 13.0.0

Edit:
I did menu.clear() in onCreateOptionsMenu before inflating the fragment's menu to solve the problem

@rddimon
rddimon commented Jul 24, 2013

I have same problem as @m190
for solve this i did:
@Override
public void onPause() {
setHasOptionsMenu(false);
super.onPause();
}
maybe another way to fix this in Watson class?

@BurntBrunch

Does the fact that the SDK now ships with a maven repository for the support library (which, frankly, I find to be an awful hack) change the project's stance on this bug?

In case you didn't know, there's a repository (that the Gradle builds get automagically) in $ANDROID_HOME/extras/android/m2repository/

@rddimon
rddimon commented Jul 24, 2013

I haven't read about Maven and Gradle). Thanks, I'll try use Marven.

@SimonVT
Collaborator
SimonVT commented Jul 24, 2013

Until either the support library is in maven central or android-maven-plugin picks up the repositories in the SDK automatically, nothing's going to happen on this issue.

@Prototik Prototik referenced this issue in Prototik/HoloEverywhere Sep 20, 2013
Closed

Menu in TabSwipeFragment #630

@philchand philchand pushed a commit to philchand/mpdroid-2014 that referenced this issue Jan 6, 2014
@abarisain abarisain Updated and patched ABS (JakeWharton/ActionBarSherlock#828) 4f8eb79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.