fix bug #10571 remove firefox app store banner on mobile devices usin…

[Ingi-Hong]

…g firefox

One-line summary

Removed firefox app store banner on iOS and Android devices using firefox.

Significant changes and points to review

Needs testing. For some reason iOS UA spoofing wasn't triggering the banner - even on the original code. Was able to succesfully test with Android UA's

• Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/112.0.0.0 Mobile Safari/537.36
• Mozilla/5.0 (Linux; Android 10; Pixel 4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/112.0.0.0 Mobile Safari/537.36
• Mozilla/5.0 (Android 4.4; Mobile; rv:70.0) Gecko/70.0 Firefox/70.0

untested:

• Mozilla/5.0 (iPhone; CPU iPhone OS 8_3 like Mac OS X) AppleWebKit/600.1.4 (KHTML, like Gecko) FxiOS/1.0 Mobile/12F69 Safari/600.1.4
• Mozilla/5.0 (iPhone; CPU iPhone OS 13_2_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.3 Mobile/15E148 Safari/604.1

as the banner wasn't triggering no matter what.

Issue / Bugzilla link

#10571

Testing

To test this work:

Unsure why spoofing UA spoofing for iOS wasn't working.

[Ingi-Hong]

I think I figured out why the banner isn't triggering. Tried using this to spoof iOS - https://addons.mozilla.org/en-US/firefox/addon/user-agent-string-switcher/, and banner still wouldn't trigger. I looked into it and even though I select the iOS option for platform, the browser still sees OS X not iOS. I also tried Chrome dev tools (sorry) to spoof iOS with the same result. I don't know how you guys test this stuff so I'll just take a step back for now.

Just as a check, I tested by commenting the ios qualifier out:

and the code works on spoofed iOS UA's.

[alexgibson]

I'll try and take a look at this soon, but spoofing the UA string might not be enough for testing here. We rely on navigator.platform as well. Testing using the iOS simulator in XCode is probably more reliable, or using a real device.

[alexgibson]

Closing stale PR. Please feel free to reopen should you find time to return to this. Thanks!

[Ingi-Hong]

I think at this point I was just waiting for a review

[alexgibson]

My apologies, it wasn't clear to me from the description that you had successfully tested that this was working or not. Feel free to reopen if you think this is ready for review

[Ingi-Hong]

Ah fair. I'll leave this closed until I can borrow a Mac from a friend!

[Ingi-Hong]

Hi Alex could you reopen this one too? Going to be testing this week.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (938dca9) 77.09% compared to head (26bc638) 76.74%. Report is 238 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13331      +/-   ##
==========================================
- Coverage   77.09%   76.74%   -0.36%     
==========================================
  Files         147      145       -2     
  Lines        7746     8006     +260     
==========================================
+ Hits         5972     6144     +172     
- Misses       1774     1862      +88     

see 23 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[alexgibson]

Closing due to inactivity

[janbrasna]

Ah the reason you're having trouble spoofing is that the iOS code actually verifies multitouch capabilities to differentiate "desktop mode" of mobile Safari from desktops with the same UA details; so you'd need to shim that drag/pointer test elsewhere…

But worry not, I'll cherry pick it to current code and test that on real devices for you, no probs — I think this is the correct way, just may need an extra test for the Client object, we'll see…