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

[Bug] horizontal scrolling can cause the address bar to appear #1506

Closed yoasif closed 4 years ago

yoasif commented 5 years ago

Steps to reproduce

  1. Navigate to https://www.cnn.com/2019/04/09/tech/locked-ipad-boy-trnd/index.html
  2. Swipe over the carousel below the video that appears on the page. Extra points if you swipe up and down while holding the swipe right to left or left to right.

Expected behavior

The browser toolbar is not shown while swiping in the carousel.

Actual behavior

The browser toolbar is shown while swiping in the carousel.

Device information

signal-attachment-2019-04-09-173439.zip

┆Issue is synchronized with this Jira Task

ekager commented 5 years ago

Thanks! I opened https://github.com/mozilla-mobile/android-components/issues/2695 to look into this issue in the toolbar behavior code

ekager commented 5 years ago

@vesta0 can we remove this as a p1 since we no longer have the hiding url bar?

vesta0 commented 5 years ago

Thanks @ekager I removed the p1 but let's keep it open to make sure this is tested once we re-introduce the dynamic nav bar.

boek commented 4 years ago

Could we test this with the top dynamic bar?

AndiAJ commented 4 years ago

Hi, still reproducible using the dynamic top bar on the latest Nightly Build #13640626 from 12/30 using the following devices:

• Google Pixel 3a (Android 9) • Huawei Mate 20 Lite (Android 8.1.0) • Samsung Galaxy S7 (Android 7) • OnePlus A3 (Android 6.0.1) • LG Nexus 4 (Android 5.1.1)

► Video 20191230_092721

sblatz commented 4 years ago

@snorp do we have the GV API's to handle this now? I don't believe the patch we worked on last week addresses this.

snorp commented 4 years ago

@snorp do we have the GV API's to handle this now? I don't believe the patch we worked on last week addresses this.

No, GV can't really do anything about this. Most likely Fenix needs to do something to detect this and not scroll the toolbar in that case.

eliserichards commented 4 years ago

@sblatz is this bug still valid or should we close it?

sblatz commented 4 years ago

I can still repro on nightly, yes.

Mugurell commented 4 years ago

After https://bugzilla.mozilla.org/show_bug.cgi?id=1627737 and https://github.com/mozilla-mobile/android-components/issues/2930 this should not reproduce anymore. Please help verify.

AndiAJ commented 4 years ago

Hi, I-ve re-checked this matter on the latest Nightly Build 200515 from 5/15 using the following devices: • Google Pixel 3a (Android 10) • Huawei Mate 20 Lite (Android 9) • OnePlus A3 (Android 6.0.1)

The Toolbar isn't displayed, but there's still a noticeable movement while swiping horizontally through the image slider. Checked it on Chrome as well, and the toolbar remains hidden without any obvious movements

► Video Fenix 20200515-095345

► Video Chrome 20200515-100909

@sblatz & @Mugurell - Please review and share your thoughts ☺️ I'll remove the QA needed label until further notice

Mugurell commented 4 years ago

@AndiAJ
From what I understand you confirm that horizontal scrolls do not anymore trigger address bar animations. This is what we wanted and based on this I'd considered this ticket as done.

The horizontals scrolls in that carousel I think indeed should not trigger a page scroll. But if this is happening, it is an issue in GeckoView for which I recommend filing a new Fenix, A-C, and Bugzilla ticket with each depending on the one below.

Mugurell commented 4 years ago

Closing as per https://github.com/mozilla-mobile/fenix/issues/1506#event-3339743972