Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

AMO thinks addons can be installed in Fenix #1134

Closed
st3fan opened this issue Mar 22, 2019 · 37 comments
Closed

AMO thinks addons can be installed in Fenix #1134

st3fan opened this issue Mar 22, 2019 · 37 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified P1 Current sprint 🙅 waiting Issues that are blocked or has dependencies that are not ready
Milestone

Comments

@st3fan
Copy link
Contributor

st3fan commented Mar 22, 2019

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 vesta0 added P2 Upcoming release needs:gv GeckoView bug required to fix the issue. See bugzilla.mozilla.org labels Apr 3, 2019
@vesta0
Copy link
Collaborator

vesta0 commented Apr 3, 2019

@cpeterso can we create a different UA for Fenix?

@vesta0 vesta0 added this to the Bugs milestone Apr 3, 2019
@cpeterso
Copy link

cpeterso commented Apr 4, 2019

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
Copy link

cpeterso commented Apr 4, 2019

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
Copy link

muffinresearch commented Apr 10, 2019

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
Copy link

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.

@kbrosnan kbrosnan added the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Apr 27, 2019
@cpeterso
Copy link

cpeterso commented May 2, 2019

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:

  • version < X is legacy Fennec
  • version >= X is Fenix (or some other GeckoView-powered browser)

The exact version number is still being bikeshedded.

@muffinresearch
Copy link

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

@reinhart1010
Copy link

SeeAlso webcompat/webcompat.com#2859

@cpeterso cpeterso removed needs:gv GeckoView bug required to fix the issue. See bugzilla.mozilla.org 🙅 waiting Issues that are blocked or has dependencies that are not ready labels May 10, 2019
@cpeterso
Copy link

@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:
MozillaReality/FirefoxReality#488

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

@cpeterso
Copy link

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.

@vesta0 vesta0 added 🐞 bug Crashes, Something isn't working, .. P1 Current sprint and removed P2 Upcoming release labels May 14, 2019
@reinhart1010
Copy link

@vesta0 vesta0 added the Release Blocker Blocks a Release label May 24, 2019
@vesta0 vesta0 added this to High priority backlog (not in the current sprint) in Fenix Sprint Kanban May 24, 2019
@cpeterso
Copy link

@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.

@bifleming bifleming moved this from High priority backlog (not in the current sprint) to Ready for Dev in Fenix Sprint Kanban May 28, 2019
@vesta0
Copy link
Collaborator

vesta0 commented May 29, 2019

@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?

@bifleming bifleming added this to Blocked by Other in Blocking Fenix Jun 12, 2019
@bifleming bifleming added this to Blocked by Other in Fenix: A-S Bugs Jun 12, 2019
@vesta0
Copy link
Collaborator

vesta0 commented Jun 12, 2019

@mheubusch please review content mentioned above.

@kbrosnan
Copy link
Contributor

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
Copy link

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
Copy link
Member

willdurand commented Jun 13, 2019

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
Copy link
Contributor

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
Copy link

@jonalmeida can this go to QA now?

@jonalmeida
Copy link
Contributor

@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
Copy link
Member

@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.

@bifleming bifleming moved this from Unnassigned to Release to Blocked items needed for Fenix Q3 in Fenix: A-S Bugs Jun 27, 2019
@jonalmeida
Copy link
Contributor

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

@bifleming bifleming removed this from Blocked items needed for Fenix Q3 in Fenix: A-S Bugs Jul 2, 2019
@vesta0 vesta0 removed this from Blocked in Fenix Sprint Kanban Jul 3, 2019
@abodea
Copy link
Member

abodea commented Jul 15, 2019

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

@abodea abodea added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Jul 15, 2019
@genodeftest
Copy link

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

@cadeyrn
Copy link
Contributor

cadeyrn commented Aug 22, 2019

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

@data-sync-user data-sync-user changed the title AMO thinks addons can be installed in Fenix FNX2-16464 ⁃ AMO thinks addons can be installed in Fenix Aug 1, 2020
@data-sync-user data-sync-user changed the title FNX2-16464 ⁃ AMO thinks addons can be installed in Fenix FNX3-14566 ⁃ AMO thinks addons can be installed in Fenix Aug 10, 2020
@data-sync-user data-sync-user changed the title FNX3-14566 ⁃ AMO thinks addons can be installed in Fenix FNX-4819 ⁃ AMO thinks addons can be installed in Fenix Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-4819 ⁃ AMO thinks addons can be installed in Fenix FNX2-16464 ⁃ AMO thinks addons can be installed in Fenix Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX2-16464 ⁃ AMO thinks addons can be installed in Fenix AMO thinks addons can be installed in Fenix May 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified P1 Current sprint 🙅 waiting Issues that are blocked or has dependencies that are not ready
Projects
No open projects
Blocking Fenix
Blocked by Other
Development

No branches or pull requests