mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.27k forks source link

AMO thinks addons can be installed in Fenix #1134

Closed st3fan closed 4 years ago

st3fan commented 5 years ago

I just saw the following feedback on a Slack channel:

Just installed Fenix, awesome, thanks! I suppose add-ons are not supported (yet?) AMO says add-ons are compatible but none can be installed.

This is probably because we use the exact same UA identification as Fennec:

Fenix: Mozilla/5.0 (Android 9; Mobile; rv:67.0) Gecko/67.0 Firefox/67.0

Firefox Mozilla/5.0 (Android 9; Mobile; rv:65.0) Gecko/65.0 Firefox/65.0

┆Issue is synchronized with this Jira Task

vesta0 commented 5 years ago

@cpeterso can we create a different UA for Fenix?

cpeterso commented 5 years ago

The Gecko team does not want GV-powered apps to customize their UA strings. However, AMO uses some privileged JS API (mozAddonManager) to talk to Fennec. The AMO team could use them to differentiate between Fennec and Fenix: no mozAddonManager -> Fenix. I will ask around for an AMO team contact.

cpeterso commented 5 years ago

This Fennec/Fenix detection issue will also affect SUMO because SUMO customizes its KB articles based on the user's OS and browser version. Perhaps we can temporarily use a custom Fenix UA string. After we complete the Fennec->Fenix migration, we can restore Fenix's standard mobile UA. I will talk to the GV team about allowing Fenix to customize the UA string.

AMO and SUMO can differentiate legacy Fennec from Fenix by browser version and the temporary custom UA. Example logic:

Browser Version? Custom Fenix UA? Detect Browser As
< 68 N/A Fennec
== 68 No Fennec
== 68 Yes Fenix
> 68 N/A Fenix
muffinresearch commented 5 years ago

Since AMO is rendered server-side, it would be much better for us to have something in the request to differentiate Fenix rather than have to use a client-side API after the page has loaded. It would be good to understand if there are any alternative options there.

cpeterso commented 5 years ago

Since AMO is rendered server-side, it would be much better for us to have something in the request to differentiate Fenix rather than have to use a client-side API after the page has loaded.

Thanks. That's good to know. I'm still talking with the Fenix and GeckoView engineers about our options. I'll keep you posted.

cpeterso commented 5 years ago

I filed a GeckoView bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1548428

The current proposal is that GeckoView 68 will spoof a special version number (like "68.99" or jump ahead to "69.0") on AMO and SUMO so the sites' server-side UA checks assume:

The exact version number is still being bikeshedded.

muffinresearch commented 5 years ago

@cpeterso I've filed https://github.com/mozilla/addons/issues/13157 to cover checking/updating our add-on compatibility code on AMO once this is solidified.

reinhart1010 commented 5 years ago

SeeAlso webcompat/webcompat.com#2859

cpeterso commented 5 years ago

@vesta0 - I removed this issue's blocked and Needs GV work labels. I resolved the GV bug as WONTFIX because Fenix can override the UA on AMO and SUMO itself like FxR does (to auto-request desktop sites):

FxR override issue: https://github.com/MozillaReality/FirefoxReality/issues/488

FxR override code: https://github.com/MozillaReality/FirefoxReality/blob/c5f1b17ed8392aff09a1cc252f78f7d01f3fc2bc/app/src/common/shared/org/mozilla/vrbrowser/browser/UserAgentOverride.java

cpeterso commented 5 years ago

In the meantime, the AMO and SUMO teams can proceed with their server-side UA checks for "Firefox Android version >= 69 means Fenix". That check will be valid even if we don't implement a UA override workaround for the overlapping period of Fennec/Fenix 68.

reinhart1010 commented 5 years ago

Duplicate: https://groups.google.com/forum/m/#!topic/fenix-nightly/ImcINWPjbB8

cpeterso commented 5 years ago

@vesta0 added this to High priority backlog (not in the current sprint)

@vesta0 - Does High priority backlog (not in the current sprint) mean this issue will be fixed before or after Fenix MVP?

This issue is only relevant while Fenix is using GV 68. There will be no point in fixing this issue after Fenix updates to GV 69.

vesta0 commented 5 years ago

@cpeterso looks like GV 69 beta will be available on July 9 so probably won't be able to ship a GV 69 build of Fenix for at least 3 weeks after MVP. @st3fan what do you think about the impact vs risk of not fixing this?

cpeterso commented 5 years ago

what do you think about the impact vs risk of not fixing this?

The impact of not fixing this bug:

These are not serious bugs. The inconsistent messaging around extension support will frustrate some users, but we can try to be up front about Fenix MVP not support extensions at this time.

This impact is probably acceptable given the short exposure period between Fenix MVP launch and then updating to GV 69.

sblatz commented 5 years ago

From reading these comments, this ticket doesn't feel eng ready. @vesta0 can you confirm this is an MVP blocker?

plwt commented 5 years ago

Without a different UA for Fenix, SUMO is going to struggle to answer questions from users as our forum tool will not be able to tell the difference between Fennec and Fenix. We will be giving users from launch the wrong information, and not helping users as they learn to use the new browser.

An example: Fenix user asks how to install an add-on. The question will be automatically directed to the Fennec support forum (instead of the Fenix forum), and if I was to look at the UA (that our forum captures), I will see they have Fennec. I will then provide the wrong information to help them and Mozilla (in particular the SUMO contributors) will not look very clever.

A different UA will mean this can be avoided.

plwt commented 5 years ago
* SUMO will show Fennec-customized content. But do we expect to have any Fenix-specialized content when Fenix MVP ships?

SUMO should have Fenix content and should have a dedicated support forum for Fenix when the MVP ships.

vesta0 commented 5 years ago

We will have a Fenix Help page. Is it possible to temporarily add a section to the Sumo Fennec page indicating "If you are using Firefox Preview, click here" and directing them to the Fenix Help page? And/or ask for the browser name to be submitted as part of a support related question?

plwt commented 5 years ago

When a user wants to ask a question in the SUMO support forum, the tool that we use in SUMO picks up the UA and directs the user towards the right forum, also providing the SUMO contributors with information telling us what product the user has. I do not believe we can make the changes mentioned in time for Firefox Preview.

cpeterso commented 5 years ago

SUMO bug to implement the server-side UA check for Fenix version >= 69.0:

https://bugzilla.mozilla.org/show_bug.cgi?id=1556520

jonalmeida commented 5 years ago

From cpeterson as well, the AMO bug to implement the service-side check: https://github.com/mozilla/addons/issues/13157

jonalmeida commented 5 years ago

The fix for this has merged, but it requires AMO/SUMO changes to verify. I'm going to leave this as blocked with QA needed for that reason.

jonichan commented 5 years ago

On SUMO, we're adding this text to every article:

If viewing a fennec article from fenix This article is for the old Firefox for Android, but you seem to be on Firefox Preview, our newest fasted, Android browser. Please visit the Firefox Preview knowledge base instead.

If viewing a Fenix article from Fennec You are using our older mobile browser, but this article is for Firefox Preview, our newest, fastest browser. Download Firefox Preview here or visit the old Firefox for Android knowledge base.

plwt commented 5 years ago

@jonichan Will that trigger based on UA?

We still need to get the UA switch implemented for the SUMO forums...

bifleming commented 5 years ago

@vesta0 @jonalmeida is the wording above all that is needed here?

vesta0 commented 5 years ago

@mheubusch please review content mentioned above.

kbrosnan commented 5 years ago

https://addons.allizom.org/ is the staging server. If the amo team's code change has been pushed to staging it can be tested there.

cpeterso commented 5 years ago

Looks like SUMO has a staging site, too: https://support.allizom.org/

Fenix's UA spoofing should probably include the AMO and SUMO staging domains.

willdurand commented 5 years ago

addons.allizom.org is the staging server. If the amo team's code change has been pushed to staging it can be tested there.

The patch has not been pushed to stage/prod yet, but it is available in -dev at: https://addons-dev.allizom.org.

jonalmeida commented 5 years ago

Fenix's UA spoofing should probably include the AMO and SUMO staging domains.

I'd prefer not to do this since we don't have a good distinction between what goes in a release/debug app for things like this. Local testing might suffice for now..

addons.allizom.org is the staging server. If the amo team's code change has been pushed to staging it can be tested there.

The patch has not been pushed to stage/prod yet, but it is available in -dev at: https://addons-dev.allizom.org.

Thanks @willdurand ! I've tested this change locally by adding the dev envirionment in a local build and I see the warning message on top. 🎉

Screenshot_1560445729

bifleming commented 5 years ago

@jonalmeida can this go to QA now?

jonalmeida commented 5 years ago

@bifleming No, I don't believe the production servers for AMO or SUMO have the required changes yet, they are only in the staging right now.

willdurand commented 5 years ago

@bifleming No, I don't believe the production servers for AMO or SUMO have the required changes yet, they are only in the staging right now.

It will land on Thursday for AMO.

jonalmeida commented 5 years ago

This can be closed now, AMO and SUMO both have their changes live. Thanks everyone for fixing this together! 🙂

abodea commented 5 years ago

Verified as fixed on the latest build 7/15 using Samsung Galaxy s10+(Android 9).

genodeftest commented 5 years ago

Is there a tracking bug or roadmap for addon support in Fenix?

cadeyrn commented 5 years ago

mozilla/addons-frontend#574 is about WebExtension support in Fenix, not this issue. ;-)