Closed Bug 1164938 Opened 9 years ago Closed 9 years ago

Enable MOZ_ANDROID_TAB_QUEUE flag only on NIGHTLY_BUILD because draw over other apps permission is required

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

All
Android
defect
Not set
normal

Tracking

(firefox39 unaffected, firefox40+ fixed, firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- unaffected
firefox40 + fixed
firefox41 --- fixed

People

(Reporter: kbrosnan, Assigned: mcomella)

References

Details

(Keywords: verifyme)

Attachments

(1 file, 1 obsolete file)

Draw over other apps permission is showing up on a 40.0a2 pave over install of 39.0a2.
Flags: needinfo?(michael.l.comella)
From conversation on irc this should block enabling updates on 40.0a2.
Component: General → Build Config & IDE Support
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Flags: needinfo?(michael.l.comella)
/r/8787 - Bug 1164938 - Add MOZ_ANDROID_TAB_QUEUE flag only on NIGHTLY_BUILD. r=nalexander

Pull down this commit:

hg pull -r 27d9d3718d0df6f13a1619d02771a5412e41982b https://reviewboard-hg.mozilla.org/gecko/
Attachment #8606037 - Flags: review?(nalexander)
Comment on attachment 8606037 [details]
MozReview Request: bz://1164938/mcomella

Hooray reviewboard.
Attachment #8606037 - Flags: review?(nalexander) → review+
Summary: Draw over other apps permission in 40.0a2 → Enable MOZ_ANDROID_TAB_QUEUE flag only on NIGHTLY_BUILD because draw over other apps permission is required
Comment on attachment 8606037 [details]
MozReview Request: bz://1164938/mcomella

Approval Request Comment
[Feature/regressing bug #]: Tab queue
[User impact if declined]:
  Users will have to accept a new permission, "Draw over other apps", to install the latest versions of Firefox (without anything to show for it!)

[Describe test coverage new/current, TreeHerder]: Tested the inverse flag locally (if ! test ...) to confirm it didn't break anything. The non-inverse flag is well tested because it's the default on Nightly, provided the check is correct.

[Risks and why]: 
  Most devs test on Nightly and local builds, where Tab queue is enabled. I'm not sure what the side effects are of disabling tab queue. In theory, we've put enough statements in the code such that nothing will happen, but we've yet to test. I'm going to get QA to verify things are working as expected.

[String/UUID change made/needed]: None
Attachment #8606037 - Flags: approval-mozilla-aurora?
Teodora, once this gets uplifted to Aurora, can you verify on Aurora that opening links from external apps works correctly (and anything else that could be intercepted by the tab queue flow)? Also, ensure the tabs queue preference (in Customize?) is missing.

On Nightly, which you probably don't need to test thoroughly, there should be no change.
Flags: needinfo?(teodora.vermesan)
Keywords: verifyme
Comment on attachment 8606037 [details]
MozReview Request: bz://1164938/mcomella

Aurora+
Attachment #8606037 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/fca5a78358ad
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Tested with:
Device: Alcatel One Touch (Android 4.1.2)
Build: Firefox for Android 40.0a2 (2015-05-17)
- A new permission is displayed under System Tools -> "Draw over other apps"
- "Open multiple links" preference is not displayed under Settings -> Customize
- Gmail links are loaded in the same tab in Aurora (Bug 1162423 - Implement Browser.EXTRA_CREATE_NEW_TAB)
- Other external links are loaded correctly
Flags: needinfo?(teodora.vermesan)
(In reply to Teodora Vermesan (:TeoVermesan) from comment #12)
> - A new permission is displayed under System Tools -> "Draw over other apps"

This is not desired - Martyn, can you see what is going on here?
Flags: needinfo?(mhaigh)
For the record, I can repro with the latest Aurora build. To do so (on vanilla Android), long-press on an app in the application list and move it to "App info" at the top of the screen when it appears.
> > - A new permission is displayed under System Tools -> "Draw over other apps"
> 
> This is not desired - Martyn, can you see what is going on here?

This has always been a caveat of having an interactive prompt display over other apps and was discussed in depth prior to a bulk of the TQ work being done. A lot of work was done to explore other options to no avail.
Flags: needinfo?(mhaigh)
(In reply to Martyn Haigh (:mhaigh) from comment #15)
> This has always been a caveat of having an interactive prompt display over
> other apps and was discussed in depth prior to a bulk of the TQ work being
> done. A lot of work was done to explore other options to no avail.

Sorry, I was unclear - this patch disables tab queue in Aurora (see comment 12) but the permission is still requested - I'll investigate.
The contacts API (READ_CONTACTS, WRITE_CONACTS, GET_ACCOUNTS) is also #ifdef NIGHTLY_BUILD [1] and appears on the Aurora build - my best guess is that Aurora is considered a NIGHTLY_BUILD (even though I think nalexander said it wasn't on IRC?). I'm confused why the preference would be disabled then [2]:

 else if (!(AppConstants.MOZ_ANDROID_TAB_QUEUE && AppConstants.NIGHTLY_BUILD) && PREFS_TAB_QUEUE.equals(key)) {
   // ... remove tab queue preference

Perhaps there's a discrepency in the AppConstants constants and build system constants?

Considering the contacts API, I'm pretty sure this will be fine when it gets down to Beta, but we should still figure it out.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidManifest.xml.in?rev=da2104421b94#56
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferences.java?rev=1fe760f5c298#778
In App Info:

"This app can access the following data on your device. To improve performance and reduce memory usage, some of these permissions are available to Aurora because it runs in the same process as Nightly."

I uninstalled Nightly and can confirm the permissions are no longer requested.

Sounds like this bug is complete.
(In reply to Michael Comella (:mcomella) from comment #18)
> "This app can access the following data on your device. To improve
> performance and reduce memory usage, some of these permissions are available
> to Aurora because it runs in the same process as Nightly."

I filed bug 1167788.
Attachment #8606037 - Attachment is obsolete: true
Attachment #8620295 - Flags: review+
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 41 → mozilla41
You need to log in before you can comment on or make changes to this bug.