Closed Bug 1819950 Opened 3 years ago Closed 3 years ago

[Experiment] [TCP CFR] The tip of the TCP CFR message doesn’t point to the lock button from the Address Bar on Android 13

Categories

(Firefox for Android :: Onboarding, defect, P2)

Firefox 111
All
Android
defect

Tracking

()

VERIFIED FIXED
113 Branch
Tracking Status
firefox110 --- disabled
firefox111 --- wontfix
firefox112 --- wontfix
firefox113 --- fixed

People

(Reporter: ctataru, Assigned: petru)

References

Details

(Whiteboard: [fxdroid])

Attachments

(5 files)

[Prerequisites]:

  • Have a local Beta build that is set-up to point to the Nimbus Prod server
  • Have the local build installed on a device.

[Steps to reproduce]:

  1. Open the local build from the prerequisites.
  2. Navigate to the Firefox "Settings" menu.
  3. Scroll down to the bottom of the list and tap the "About Firefox Beta" option.
  4. Tap the "Firefox Browser" logo 5 times and go back to the "Settings" menu.
  5. Scroll down to the bottom of the list and tap the "Secret Settings" option.
  6. Enable the “Use Nimbus Preview Collection” option and restart the browser.
  7. Navigate again to the Firefox "Settings" menu -> "About Firefox Beta" option.
  8. Tap the "Firefox Browser" logo 5 times and go back to the "Settings" menu.
  9. Scroll down to the bottom of the list and tap the "Nimbus Experiments" option.
  10. Tap the "Firefox Android TCP CFR Experiment" experiment.
  11. Tap the “treatment-a” branch and restart the browser.
  12. Navigate to a website and observe the TCP CFR message.

[Expected results]:

  • The tip of the TCP CFR message points to the lock button from the Address Bar.

[Actual results]:

  • The tip of the TCP CFR message does not point to the lock button from the Address Bar and overlaps the lock button.

[Device name]:
Samsung galaxy Tab A8

[Android version]:
Android 13

[Firefox release type]:
Firefox Beta

[Firefox version]:
107.0b7

[Additional Notes]:

  • When the orientation of the page it’s changed from landscape to portrait the TCP CFR message appears on different parts of the page.
  • I think that this issue is only reproducible on Android 13 devices.

This is an ugly bug, but I don't think it needs to block TCP rollout in 111. We have a fix, we should consider uplifting it to a 111 dot release.

Severity: -- → S3
Priority: -- → P2
Whiteboard: [fxdroid]

Thanks for the screen recording! It makes understanding the bug much easier.

This bug might be related to bug 1809592.

For anyone trying to reproduce this bug, you don't need to enroll in the Nimbus experiment. You can install Firefox Nightly and clear its app data in system Settings > Apps > Firefox Nightly > Storage > Clear data.

See Also: → 1809592

For reference, both bug 1813427 and bug 1809592 touch on this problem (but both point to the same refactor Petru did for CFRs in general).

Questions for whoever picks this up:

  • Is this a regression from Petru's CFR refactor?
  • If not: how dependable is the placement logic for this CFR? How likely is it that there are other devices that might have a similar bug?
Attached image image.png

The TCP CFR is correctly positioned for me on a Samsung Tab A - SM-T585 (screenshot added as an attachment) and it was already verified on other devices also in https://bugzilla.mozilla.org/show_bug.cgi?id=1813427.

Based on the above and the reporter's screenshot I think the presented issue is not a regression but an edgecase that affects all CFRs and it depends on a new Samsung tablets feature - the taskbar[1].
Not sure whether the Samsung taskbar is enabled by default or has to be enabled by the user.
The issue seems to be that the CFR does not account for the taskbar height so it should be easy to fix but I don't have a tablet with this feature available to test.

[1] https://www.sammobile.com/news/one-ui-feature-focus-samsung-new-taskbar/

@Carmen Can you give more details about what's needed to reproduce the issue?
Don't have a tablet with Android 13 to test but trying the feature on emulators I see everything looking okay.
I'm curious on whether rotating the screen is necessary to trigger the issue?

Flags: needinfo?(ctataru)

(In reply to Petru-Mugurel Lingurar [:petru] from comment #5)

@Carmen Can you give more details about what's needed to reproduce the issue?
Don't have a tablet with Android 13 to test but trying the feature on emulators I see everything looking okay.
I'm curious on whether rotating the screen is necessary to trigger the issue?

Hi Petru!
I have managed to reproduce the issue after I have updated my device to Android 13. In order to reproduce the issue you need to rotate the screen.
I have also removed the device's toolbar, and I can still reproduce the issue. I Have also changed the device font/dimension and I can still reproduce the issue.
However, I am not sure if this issue is specific only to this device, we have also tried to reproduce this issue on a emulator (Virtual Nexus 10 tablet with Android 13) and didn't managed to reproduce it.

Flags: needinfo?(ctataru)

Thank you for the update.
Looked to me also and I understand you confirmed that the CFR is positioned correctly when it is being shown but it is not automatically dismissed when the screen rotates.
(We know of this issue with improper placement after the screen being rotated, that's why most of the popups and menus should've been closed upon screen rotation).
This leads me to believe that all CFRs are affected, not just the TCP one. Will investigate why they are not automatically dismissed upon screen rotation.

Assignee: nobody → petru.lingurar
Status: NEW → ASSIGNED

Seems like on newer Android versions DisplayManager can give erroneous results.
Did not find this properly documented or a clear explanation as to why but I found in our code a mention about this[1] which led me to bug 1806072 about a similar issue to the one here.

[1] https://searchfox.org/mozilla-central/rev/8ed22fcd56968c95a73a6c82b42f732f01a4bdae/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java#658-660

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: qe-verify+
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch

This issue is verified fixed.

This change looks too big to uplift to a 111 dot release.

Flags: qe-verify+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: