Closed
Bug 1372926
Opened 7 years ago
Closed 7 years ago
[Android O] Add to Home screen doesn't work on any version of Firefox on Oreo
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(fennec+, firefox54 wontfix, firefox55 wontfix, firefox56 wontfix, firefox57 fixed, firefox58 fixed)
RESOLVED
FIXED
Firefox 58
People
(Reporter: marcia, Assigned: snorp)
References
Details
Attachments
(2 files)
(deleted),
text/x-review-board-request
|
cnevinchen
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-review-board-request
|
cnevinchen
:
review+
|
Details |
Seen while running the latest Android O beta, Version 8.0.0, running on a Google Pixel.
STR:
1. Load a web page
2. Page | Add to Home Screen
The page is not added. I confirmed that the same operation works using Google Chrome.
Updated•7 years ago
|
QA Contact: ioana.chiorean
Comment 1•7 years ago
|
||
Changes are documented here: https://developer.android.com/preview/behavior-changes.html#as
Comment 4•7 years ago
|
||
From https://bugzilla.mozilla.org/show_bug.cgi?id=1378348#c0 also se elink in #c1
> The com.android.launcher.action.INSTALL_SHORTCUT broadcast no longer has any
> effect on your app, because it is now a private, implicit broadcast. Instead,
> you should create an app shortcut by using the requestPinShortcut() method from
> the ShortcutManager class.
Updated•7 years ago
|
Depends on: build-android-o
Comment 5•7 years ago
|
||
(In reply to Ioana Chiorean from comment #4)
> From https://bugzilla.mozilla.org/show_bug.cgi?id=1378348#c0 also se elink
> in #c1
>
> > The com.android.launcher.action.INSTALL_SHORTCUT broadcast no longer has any
> > effect on your app, because it is now a private, implicit broadcast. Instead,
> > you should create an app shortcut by using the requestPinShortcut() method from
> > the ShortcutManager class.
sebastian: This requires us to _target_ Android 25+ and not just build with Android 25+, right?
Flags: needinfo?(s.kaspari)
Comment 6•7 years ago
|
||
The new API that the "behavior change" documentation[1] mentions is API 26:
https://developer.android.com/reference/android/content/pm/ShortcutManager.html#requestPinShortcut(android.content.pm.ShortcutInfo,%20android.content.IntentSender)
Compiling with the new SDK is enough for that. There are some other things that fall apart once we actually target 26.
[1] https://developer.android.com/preview/behavior-changes.html#as
Flags: needinfo?(s.kaspari)
Comment 7•7 years ago
|
||
we probably need to temporarily disable / hide "Add to homescreen" on Android O if a proper solution to compile with the new SDK requires gradle build and all the other dependencies
Comment 8•7 years ago
|
||
(In reply to Joe Cheng [:jcheng] (please needinfo) from comment #7)
> we probably need to temporarily disable / hide "Add to homescreen" on
> Android O if a proper solution to compile with the new SDK requires gradle
> build and all the other dependencies
That's what we did for the Focus 1.2 release too.
Although note that I do think that we could update to the new SDK without waiting for gradle first (nalexander might know more here though; I think he might even tried that locally). We did updates of the SDK in the past and while it's a bit more complicated than changing a line in gradle it's doable.
We couldn't update to the N SDK that easily because it required a new compiler. But this restriction is now gone in Android O again and an SDK update could work.
Let me know if you want to do that and whether you need my help. Also note that every past SDK update required additional work for fixing behavior changes and API deprecation too.
Comment 9•7 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #8)
> (In reply to Joe Cheng [:jcheng] (please needinfo) from comment #7)
> > we probably need to temporarily disable / hide "Add to homescreen" on
> > Android O if a proper solution to compile with the new SDK requires gradle
> > build and all the other dependencies
>
> That's what we did for the Focus 1.2 release too.
>
> Although note that I do think that we could update to the new SDK without
> waiting for gradle first (nalexander might know more here though; I think he
> might even tried that locally). We did updates of the SDK in the past and
> while it's a bit more complicated than changing a line in gradle it's doable.
> We couldn't update to the N SDK that easily because it required a new
> compiler. But this restriction is now gone in Android O again and an SDK
> update could work.
Sadly, I believe there's a snag (see also https://bugzilla.mozilla.org/show_bug.cgi?id=1384312#c0). I believe that SDK 25+ requires build-tools 25.0.3, which requires... glibc 2.14+. Our ancient CentOS 6-based build images are pinned to glibc 2.12, and I believe it's not possible to update glibc and not feasible to have two glibc versions installed.
I got pretty far along the path of replacing the CentOS 6-based build images with Debian based build images that worked with glibc 2.14+, but haven't pushed it across the line. Moving to CentOS 7-based build images might be quite simple, since the centos6-build Dockerfile has been improved recently.
If SDK 25+ doesn't require a more recent build-tools (perhaps that's Gradle only?), then we can absolutely push forward the SDK versions for building, etc. We will need to fix the Proguard issue, but that's not difficult -- see https://bugzilla.mozilla.org/show_bug.cgi?id=1352599.
Updated•7 years ago
|
Reporter | ||
Comment 10•7 years ago
|
||
(In reply to Joe Cheng [:jcheng] (please needinfo) from comment #7)
> we probably need to temporarily disable / hide "Add to homescreen" on
> Android O if a proper solution to compile with the new SDK requires gradle
> build and all the other dependencies
There are online reports that Pixel devices will be updated to Android O on 8/21, not sure if that is true or not.
Comment 11•7 years ago
|
||
Our partners have started to discover this on Android O.
If we are not going to be able to fix this any time soon, we need to remove the add to home screen option for Android O.
Reporter | ||
Comment 12•7 years ago
|
||
Android O shipped to Pixels the other day - do we want to create a separate bug to track removing the home screen option?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8900448 [details]
Bug 1372926 - Support new shortcut API in Android O
https://reviewboard.mozilla.org/r/171804/#review177398
::: mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java:14
(Diff revision 2)
> import android.app.Application;
> import android.content.ContentResolver;
> import android.content.Context;
> import android.content.Intent;
> +import android.content.pm.ShortcutManager;
> +import android.content.pm.ShortcutInfo;
This is added in API leve 25
https://developer.android.com/reference/android/content/pm/ShortcutManager.html
I think we need to update our SDK before we can use it.
I believe there's a discussion between Max and Nick about what's the best way do this. AFAIK it's more complicated than previous update. But I don't know the schedule about it.
Comment 16•7 years ago
|
||
> I believe there's a discussion between Max and Nick about what's the best
> way do this. AFAIK it's more complicated than previous update. But I don't
> know the schedule about it.
The tricky thing (I think) is requiring newer build-tools, which in turn sets off a cascade. See https://bugzilla.mozilla.org/show_bug.cgi?id=1370119#c2.
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8900448 [details]
Bug 1372926 - Support new shortcut API in Android O
https://reviewboard.mozilla.org/r/171804/#review179874
Please request review again once we've done the upgrade :)
Attachment #8900448 -
Flags: review?(cnevinchen)
Let's put "Oreo" in the title to make this searchable.
Summary: [Android O] Add to Home screen doesn't work on any version of Firefox → [Android O] Add to Home screen doesn't work on any version of Firefox on Oreo
Comment 21•7 years ago
|
||
We need a patch for Firefox 56 that at least removes this menu poption for Android O.
If an OEM ships Android O, we need the functionality to not be broke.
I believe the patch to remove the menu option is in bug 1394356 and should be in the beta 11 build that we'll do later today.
If we need this fix for 56, then please try to get review and testing in place quickly so that we can uplift this to beta. Sebastian, Joe, what do you think? Can anyone work on this now? Or is it going to wait for Firefox 57?
Comment 23•7 years ago
|
||
It would be best to get this into 56 indeed.
Comment 24•7 years ago
|
||
Flagging for the mobile triage meeting tomorrow - hopefully someone from the mobile core team can pick this up.
tracking-fennec: --- → ?
Flags: needinfo?(s.kaspari)
Comment 25•7 years ago
|
||
agree with Andreas on comment 23.
flagging Wesly
Flags: needinfo?(jcheng) → needinfo?(wehuang)
Comment 26•7 years ago
|
||
This patch can't go in 56 because it requires build changes (I think).
For 56, we took bug 1394356 which removes add to homescreen on Android O.
Comment 27•7 years ago
|
||
I meant to apply the temporary solution bug 1394356 for 56 and it looks like we just did.
The full solution will require us to apply gradle build and SDK update first
Comment 28•7 years ago
|
||
Joe is correct, the plan is to have 1394356 (remove the menu option from UI) for 56+, and support the full "Add to Home screen" functionality after gradle is ready. Set P1 for now.
(In reply to Joe Cheng [:jcheng] (please needinfo) from comment #27)
> I meant to apply the temporary solution bug 1394356 for 56 and it looks like
> we just did.
> The full solution will require us to apply gradle build and SDK update first
Flags: needinfo?(wehuang)
Priority: -- → P1
Comment 29•7 years ago
|
||
[triage0913] + as P1.
Updated•7 years ago
|
tracking-fennec: ? → +
Comment 30•7 years ago
|
||
To be 100% clear, we're talking about temporarily disabling "add to home screen" in 56 (and 57) running on Oreo, while still keeping it working as expected in older Android versions, right?
Comment 31•7 years ago
|
||
> To be 100% clear, we're talking about temporarily disabling "add to home screen" in 56 (and 57) running on Oreo, while still keeping it working as expected in older Android versions, right?
Yes. This was done in bug 1394356.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 35•7 years ago
|
||
The attached patch will allow us to restore the "add to home screen" functionality even without the SDK upgrade.
Comment 36•7 years ago
|
||
\o/
Comment hidden (mozreview-request) |
Comment 38•7 years ago
|
||
mozreview-review |
Comment on attachment 8900448 [details]
Bug 1372926 - Support new shortcut API in Android O
https://reviewboard.mozilla.org/r/171804/#review189650
::: mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java:562
(Diff revision 5)
>
> private static void createHomescreenIcon(final Intent shortcutIntent, final String aTitle,
> final String aURI, final Bitmap aIcon) {
> + final Context context = GeckoAppShell.getApplicationContext();
> +
> + if (Versions.feature26Plus) {
nit: can we move this code to an utility class and let it decide the implemetation there ?
Attachment #8900448 -
Flags: review?(cnevinchen) → review+
Comment 39•7 years ago
|
||
mozreview-review |
Comment on attachment 8912801 [details]
Bug 1372926 - Revert bug 1394356, allowing "add to homescreen" again on Android O
https://reviewboard.mozilla.org/r/184110/#review189652
Attachment #8912801 -
Flags: review?(cnevinchen) → review+
Comment 40•7 years ago
|
||
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/787e7de1ace6
Revert bug 1394356, allowing "add to homescreen" again on Android O r=nechen
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ac21f869569
Support new shortcut API in Android O r=nechen
Comment 41•7 years ago
|
||
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bb669555e24
Followup to fix lint failures r=me
Bugherder might butcher this due to "revert" being in one of the commit messages, but here's the first patch merged to m-c:
https://hg.mozilla.org/mozilla-central/rev/787e7de1ace6
Comment 43•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2ac21f869569
https://hg.mozilla.org/mozilla-central/rev/6bb669555e24
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
These patches conflicted with the ones from bug 1395841 when they got merged together. I think I resolved the conflicts, but it wouldn't hurt to make sure I did it right.
Comment 45•7 years ago
|
||
What is the plan wrt to 57 for this bug?
thanks
Reporter | ||
Comment 46•7 years ago
|
||
I just tested this using the latest nightly and it works - using Page | Add Page Shortcut.
Assignee | ||
Comment 47•7 years ago
|
||
Comment on attachment 8900448 [details]
Bug 1372926 - Support new shortcut API in Android O
Approval Request Comment
[Feature/Bug causing the regression]: Android N release
[User impact if declined]: Unable to add shortcuts to homescree on Android N
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Ensure Page -> "Add to homescreen" works on an Android N device.
[List of other uplifts needed for the feature/fix]: Both patches on this bug
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only adds special path for Android N devices, others unaffected.
[String changes made/needed]: None
Attachment #8900448 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 48•7 years ago
|
||
Note to relman: this is also an important partner issue. Contact Mike Kaply for details.
Updated•7 years ago
|
Assignee: nobody → snorp
Comment 49•7 years ago
|
||
Comment on attachment 8900448 [details]
Bug 1372926 - Support new shortcut API in Android O
Make senses anyway. Taking it.
Should be in 57b6
Attachment #8900448 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 50•7 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•