1137928 - Share overlay unexpectedly appears in recent app stack on Android L (and later)

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Overlays, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

1) Menu -> share -> Add to Firefox
2) Hit home
3) Hit recent apps

Expected: no add to firefox
Actual: add to firefox

Comment 1

[Richard Newman [:rnewman]]

Activity should be marked as excludeFromRecents.

http://developer.android.com/reference/android/R.attr.html#excludeFromRecents

This should go in the manifest.

Comment 2

[zclouse11]

I Would be interested in working on this bug. Where do you think I should start?

Comment 3

[Richard Newman [:rnewman]]

Start by building Fennec, and seeing if you can reproduce via the steps in Comment 0.

https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

Once you've got to that point, take a look at 

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidManifest.xml.in#382

and Comment 1 should be enough for you to make a change, rebuild/repackage/reinstall, and see if you can still reproduce.

Comment 4

[Bhargav Chippada [:bhargavch]]

Hi, I would like to work on this bug. I have build the fennec, i will try to reproduce the bug.

Comment 5

[Bhargav Chippada [:bhargavch]]

Attachment: Patch_[Bug_1137928]_share_overlay_in_activity_stack.patch

Hi, I have reproduced the bug and created a patch for it. I tested the patch, it's working :) waiting for review

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, zclouse11 - do you already have a patch created? If so, please upload it! If not, I'll pass this bug onto Bhargav and ask that you comment on another open [good first bug] to get working there (may I recommend bug 1106903? The previous contributor has been inactive long enough that I can just assign you over there - comment on the bug if you'd like to work on it). Sorry about that!

Bhargav, thanks for the patch! We like to prioritize assignees by the order in which they ask to be assigned to the bug so please make sure you read the previous comments to ensure no one else is already working on a bug before working on it and posting a patch - I'm only making an exception here because you already put in the work to create a patch (but this may not always be the case!).

Comment 7

[Bhargav Chippada [:bhargavch]]

Hi, I am very sorry for that. I am new to this so didn't know the procedure. Thanks for telling me :), can you also suggest me some good first or second bugs to work on?

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Perhaps you could work on bug 1130372? Comment over there and you'll be assigned.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

zclouse11, let me know if you want another bug to work on - I'm going to give this one to Bhargav.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8571397 [details] [diff] [review]
Patch_[Bug_1137928]_share_overlay_in_activity_stack.patch

Review of attachment 8571397 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me although your patch is in the incorrect format - you should be able to run `hg export -g > <filename>` to get your patch in an acceptable format to a file called <filename>.

Can you upload a new patch in the correct format, Bhargav?

Comment 11

[Bhargav Chippada [:bhargavch]]

Attachment: Patch_[Bug_1137928]_share_overlay_in_activity_stack_r=mcomella.patch

Hi Michael, I am attaching the patch in the format u specified :) Is it ok now?

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8574280 [details] [diff] [review]
Patch_[Bug_1137928]_share_overlay_in_activity_stack_r=mcomella.patch

Review of attachment 8574280 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like it's missing some metadata that other patches usually have but it successfully applies so it's fine with me: r+.

Here's a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3b988a7aaa8

Once the push goes green, add the checkin-needed keyword.

Thanks, Bhargav!

Comment 13

[Bhargav Chippada [:bhargavch]]

(In reply to Michael Comella (:mcomella) from comment #12)
> Comment on attachment 8574280 [details] [diff] [review]
> Patch_[Bug_1137928]_share_overlay_in_activity_stack_r=mcomella.patch
> Here's a try push:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3b988a7aaa8
> 
> Once the push goes green, add the checkin-needed keyword.
> 
> Thanks, Bhargav!

Thanks! :) try push is complete, everything is green except one unclassified issue i guess. what is it? If everything is okay then checkin-needed can be added.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Bhargav Chippada [:bhargavch] from comment #13)
> Thanks! :) try push is complete, everything is green except one unclassified
> issue i guess. what is it? If everything is okay then checkin-needed can be
> added.

It looks like it may be an intermittent issue (it matches one of the errors at the bottom of the page after you click on "rc2") so I re-ran the test to find out. If it still doesn't go green, let me know.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

By the way, Bhargav, it would be good to uplift this to Aurora where the share overlay will be getting some publicity - can you go to the patch details page and request approval for uplift to Aurora? You'll have to fill in a bit of information about the bug - let me know if you have any questions.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Actually, I noticed this occurs only on Android L and appears to be a system-wide change - gmail and other apps exhibit a similar behavior. Unfortunately, this patch doesn't appear to fix the issue. There may be a new flag to fix this.

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Sorry about this Bhargav - it isn't your fault! I didn't expect this to be an Android L specific issue.

It looks like the "noHistory" flag fixes the issue, but at the cost of closing the dialog when the app switcher is open; I'm unsure how this affects KitKat-.

Perhaps this is just a 5.0 bug fixed in 5.1?

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

If you have an Android L device, feel free to explore, Bhargav, but you don't have to finish this.

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Bhargav, I'd like to get this into the Firefox 38 channel (currently Aurora) so this should be done soon - let me know if you want to finish this up, otherwise I'm going to take over early next week.

Again, you don't have to if you don't want to (or don't have an Android L device!).

Tips: liuche recommmended looking into taskAffinity:

https://developer.android.com/guide/topics/manifest/activity-element.html#aff
https://stackoverflow.com/questions/17872989/android-task-affinity-explanation

Comment 20

[Bhargav Chippada [:bhargavch]]

Sorry for the late reply. I did some testing on android L (5.0.2) and kitkat. Here are the results:

excludeFromRecents tag:
  1) Lollipop
     Comment 0 steps: Doesn't show in recent activity stack but closes the dialog
     app switcher: shows 'add to fennec' in recent activity stack
  2) Kitkat
     works as expected (no closing of dialog too)

noHistory tag:
  1) Lollipop
     Comment 0 steps: Doesn't show in recent activity stack but closes the dialog
     app switcher: Doesn't show in recent activity stack but closes the dialog
  2) Kitkat
     Doesn't work as expected because dialog is closed when app switcher is open or by comment 0 steps.

Yes, I think it is android L specific issue because i don't see this bug in kitkat. noHistory tag will affect kitkat-. excludeFromRecents tag is behaving like i described above which is not fulfilling.

I will uplift this to aurora, check my answers, are they ok? also can u please tell me more about what aurora is?

Approval Request Comment
[Feature/regressing bug #]: 1137928
[User impact if declined]: Android L specific problem, 'add to fennec' is shown in recent activity stack and clicking on fennec activity will not show the 'add to fenenc' dialog in it, the dialog is still in the recent activity stack.
[Describe test coverage new/current, TreeHerder]: 1 unclassified issue for Android 2.3 API9 opt
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3b988a7aaa8
[Risks and why]: 'add to fennec' dialog is shown in recent activity stack separated from fennec activity, it is not killed even if the actual fennec activity is killed.
[String/UUID change made/needed]:

Comment 21

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Sorry for the delay, Bhargav.

(In reply to Bhargav Chippada [:bhargavch] from comment #20)
> Sorry for the late reply. I did some testing on android L (5.0.2) and
> kitkat. Here are the results:

Thanks for the thorough testing!

> excludeFromRecents tag:
>   1) Lollipop
>      Comment 0 steps: Doesn't show in recent activity stack but closes the
> dialog
>      app switcher: shows 'add to fennec' in recent activity stack
>   2) Kitkat
>      works as expected (no closing of dialog too)

I think the behavior is the same as without this flag (and thus this patch).

Have you looked into taskAffinity at all?

Note that this might be unfixable, as per comment 16.

> I will uplift this to aurora, check my answers, are they ok? also can u
> please tell me more about what aurora is?

Our releases are divided into four channels: release, beta, aurora, and nightly; code that lands on fx-team (or mozilla-central) gets put into that day's Nightly release. Going left-to-right, each channel is considered more stable than the previous - users can download whichever channel they want to get the amount of stability they desire. We look for bugs in each channel so each channel helps us shake out bugs. Since more stable channels tend to have more users, more bugs are usually uncovered as the code moves from channel to channel.

To uplift to aurora means to land the code on the aurora channel so users on the aurora release will get those changes. We generally try not to uplift, but we do so when there are critical bugs or incomplete features that need finishing.

Note: when you uplift to aurora, make sure you set the flag on the attachment itself.

However, this uplift is unnecessary since we won't be landing this patch on fx-team.

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I can confirm that neither:

android:taskAffinity=""

android:allowTaskReparenting="false"

works. I'm going to say Android L changed how the task stack works and they didn't bother updating the docs.

Further research, which I don't have time to do now, would likely require digging into the Android source code to see what they changed.

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Unassigning to inactivity.

Martyn did some work in bug 1155911 on a similar issue for tab queues - perhaps that information would be useful here?

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Also, maybe the work in bug 1165856 will also be useful here.

Comment 26

[Jan Henning [:JanH]]

As per bug 1498628, this also affects later Android versions, although apparently not always to the same extent, and it also depends on how the calling app exactly goes about launching our share dialogue.

Comment 27

[Daniel]

Some more details from investigating bug 1498628:

It only affects some third-party apps. E.g. sharing links from the new Google News  app (formerly Playstand) always reproduces this issue. Sharing from Google Photos also triggers this bug.

Other apps like Google Maps and Google Play Store aren't affected.

Comment 28

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.