Tab Queue issues with "Draw over other apps" disabled

VERIFIED FIXED in Firefox 47

Status

()

Firefox for Android
General
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: TeoVermesan, Assigned: sebastian)

Tracking

(Blocks: 1 bug, {regression})

46 Branch
Firefox 48
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 wontfix, firefox47+ fixed, firefox48+ verified)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Steps to reproduce:
1. Go to Settings -> Apps -> Draw over other apps -> Aurora/Nightly and disable the option
2. Open Firefox and go to Settings -> General and tap "Open multiple links" option
3. From the "Open multiple links" dialog tap on Settings (to enable 'Draw over other apps'
4. Enable the option
5. Go back

Expected results:
- "Open multiple links" option is enabled in Settings

Actual results: 
- "Open multiple links" dialog is still displayed
- The option is not enabled

Note;
- I am not able to reproduce this every time 
- Please take a look at the following video: https://www.youtube.com/watch?v=CYJaY_Ldn7Q&feature=youtu.be

Also:
- sometimes with "Draw over other apps" disabled, with a clean Aurora profile, go to Settings -> General and turn on "Open multiple links" option => the option is enabled. No dialog to turn on "Draw over other apps" is shown.
- please take a look at the following video: https://youtu.be/eOsRBy8WLrM
(Assignee)

Updated

2 years ago
Assignee: nobody → s.kaspari
Blocks: 1205216, 1239284
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
(In reply to Teodora Vermesan (:TeoVermesan) from comment #0)
> - sometimes with "Draw over other apps" disabled, with a clean Aurora
> profile, go to Settings -> General and turn on "Open multiple links" option
> => the option is enabled. No dialog to turn on "Draw over other apps" is
> shown.
> - please take a look at the following video: https://youtu.be/eOsRBy8WLrM

I have seen this a couple of times today too. This is really strange. ContextCompat.canDrawOverlays() seems to return true while the toggle is actually off. This almost looks like a bug in the Android system. Actually trying to draw over other apps fails in this situation.
Currently I can't get this in an unbroken state again on my device - even re-install, clear data, reboot does not fix this.
(Assignee)

Comment 2

2 years ago
This looks a lot like this:
https://code.google.com/p/android/issues/detail?id=198671

If we are hitting this then the return value of canDrawOverlays() is influenced by other Firefox installations on the device (e.g. if Nightly can draw over other apps then we might not show the prompt in Aurora). This needs some more investigation.
(Assignee)

Comment 3

2 years ago
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> If we are hitting this then the return value of canDrawOverlays() is
> influenced by other Firefox installations on the device (e.g. if Nightly can
> draw over other apps then we might not show the prompt in Aurora). This
> needs some more investigation.

Yeah, in my case I could "fix" the issue by enabling or disabling overdraw for all installed Firefox versions.

@Teodora: Do you have multiple Firefox versions installed on the device? Do you still see the issue if all Firefox versions are using the same setting?
Flags: needinfo?(teodora.vermesan)
(Reporter)

Comment 4

2 years ago
I have both Nightly and Aurora installed on this device. 
Scenarios:

1. First both versions have "Draw over other apps" set to "No":
-On Aurora I try to enable "Open multiple links" feature: but the dialog is still displayed and the option is not enabled. "Draw over other apps" is set to "Yes".
-On Nightly works ok. "Draw over other apps" is set to "Yes" also.

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Nightly and works ok
-Then tried with Aurora: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Aurora and "Yes" to Nightly.

2. Restart the device and repeat the above steps with the other version first:

Both versions have "Draw over other apps" set to "No":
-On Nightly I try to enable "Open multiple links" feature: but the dialog is still displayed and the option is not enabled. "Draw over other apps" is set to "Yes"
-On Aurora works ok. "Draw over other apps" is set to "Yes", also

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Aurora and works ok.
-Then tried with Nightly: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Nightly and "Yes" to Aurora
Flags: needinfo?(teodora.vermesan)
(Assignee)

Comment 5

2 years ago
Unfortunately it seems like we won't be able to fix that - at least until the Android team fixes this bug:
https://code.google.com/p/android/issues/detail?id=198671

This is caused by us using "sharedUserId" and then Android mixing up the settings. Even though it seems like we do not need "sharedUserId" anymore, we can't get rid of it because this would revoke access to our own files:
https://code.google.com/p/android/issues/detail?id=1227
https://code.google.com/p/android/issues/detail?id=14074

Android never implemented a "downgrade path" for removing or changing a sharedUserId of an existing application.
(Assignee)

Comment 6

2 years ago
(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> Unfortunately it seems like we won't be able to fix that - at least until
> the Android team fixes this bug:
> https://code.google.com/p/android/issues/detail?id=198671

There seems to be no movement in this bug at all.

As a last resort we could try to not use Settings.canDrawOverlays() and instead try to add an invisible overlay: If successful -> We can add overlays. Remove it again. If exception: Seems like we do not have permission.
(Assignee)

Comment 7

2 years ago
I looked at the code again and added a comment to the bug report:
https://code.google.com/p/android/issues/detail?id=198671#c7

This method is obviously broken for apps that use sharedUserId. I implemented the workaround mentioned in comment 5. This seems to work just fine. I'll test some more and then push it to review.

Another option would be to use reflection to access Settings.isCallingPackageAllowedToDrawOverlays() and provide it with the correct package name. But this is prone to break if this non-public API changes.
(Assignee)

Comment 8

2 years ago
Created attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Review commit: https://reviewboard.mozilla.org/r/43193/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43193/
Attachment #8736275 - Flags: review?(ahunt)
(Assignee)

Comment 9

2 years ago
Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43193/diff/1-2/

Comment 10

2 years ago
Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

https://reviewboard.mozilla.org/r/43193/#review40105
Attachment #8736275 - Flags: review?(ahunt) → review+
(Assignee)

Comment 11

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/a70b50e2830f3316d5a1307bb9f8231ca88efc52
Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r=ahunt

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a70b50e2830f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
(Assignee)

Updated

2 years ago
Flags: needinfo?(s.kaspari)
(Assignee)

Comment 13

2 years ago
Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

We should uplift this. I somehow forgot to request this.

Approval Request Comment

[Feature/regressing bug #]: Runtime permissions - Introduced with Firefox 46.0 - Bug 1212830. Maybe it's too late for 46, but requesting uplift anyways.

[User impact if declined]: If multiple versions of Firefox are installed locally (say Release and Beta) then the prompt for "Draw over other apps" might not be shown when it should or shown even if we already have the permission.

[Describe test coverage new/current, TreeHerder]: Local testing with multiple installations.

[Risks and why]: Low - This has been in Nightly for two weeks now.

[String/UUID change made/needed]: -
Flags: needinfo?(s.kaspari)
Attachment #8736275 - Flags: approval-mozilla-beta?
Attachment #8736275 - Flags: approval-mozilla-aurora?
Keywords: regression
I think this can wait till 47. We are very close to the 46 release, this is a very particular case (having more than one version installed) and doesn't reproduce every time.
status-firefox46: --- → wontfix
status-firefox47: --- → affected
tracking-firefox47: --- → +
tracking-firefox48: --- → +
Attachment #8736275 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Hi Teodora, could you please verify this bug is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(teodora.vermesan)
Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Recent regression, was manually verified by dev, Aurora47+
Attachment #8736275 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 17

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/5b3bc1699878
status-firefox47: affected → fixed
backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=2430277&repo=mozilla-aurora
status-firefox47: fixed → affected
Flags: needinfo?(s.kaspari)
(Reporter)

Comment 19

2 years ago
Based on steps from comment 4:

1. First both versions have "Draw over other apps" set to "No":
-On Aurora: affected
-On Nightly works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Nightly and works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed
-Then tried with Aurora: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Aurora and "Yes" to Nightly.

2. Restart the device and repeat the above steps with the other version first:

Both versions have "Draw over other apps" set to "No":
-On Nightly works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed
-On Aurora works ok. "Draw over other apps" is set to "Yes", also

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Aurora and works ok. 
-Then tried with Nightly: and works ok. 

3. "Draw over other apps" set to "Yes" for Aurora and "No" for Nightly
-On Nightly works ok.


Verified as fixed using:
Device: Nexus 6 (Android 6.0)
Build: Firefox for Android 48.0a1 (2016-04-17)
status-firefox48: fixed → verified
Flags: needinfo?(teodora.vermesan)
(Assignee)

Comment 20

2 years ago
Created attachment 8744348 [details] [diff] [review]
1244722-TQ-Aurora.patch

Modified patch for Aurora (Fixing imports)
Flags: needinfo?(s.kaspari) → needinfo?(cbook)

Comment 21

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/892d0f1308f8
status-firefox47: affected → fixed
(In reply to Sebastian Kaspari (:sebastian) from comment #20)
> Created attachment 8744348 [details] [diff] [review]
> 1244722-TQ-Aurora.patch
> 
> Modified patch for Aurora (Fixing imports)

landed by ryan
Flags: needinfo?(cbook)
Verified based on comment 19
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.