This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Wait until a response from GV to animate the toolbar #10555

Closed

Mugurell opened this issue Jul 5, 2021 · 1 comment

Assignees

Labels

<toolbar>

Contributor

Mugurell commented Jul 5, 2021 •

edited by data-sync-user

Loading

Issues reported on Fenix - mozilla-mobile/fenix#16352

Seems to be a race between us handling a new touch here and GV informing us about how it handled the touch here.

If we are to win it based on the default InputResultDetail we would preemptively expand the toolbar.

Seems like we need another status than INPUT_UNHANDLED to know that we don't yet have a status from GeckoView about how it handled the touch.
I propose using a new INPUT_HANDLING_UNKNOWN as the default used in InputResultDetail based on which we can avoid any updates to the toolbar until we actually get INPUT_UNHANDLED / INPUT_HANDLED / INPUT_HANDLED_CONTENT.

┆Issue is synchronized with this Jira Task

The text was updated successfully, but these errors were encountered:

Mugurell self-assigned this

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

3e6e72d

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

edcb765

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

Mugurell added the <toolbar> label

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

eaccbfa

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

fca33ab

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - BrowserToolbarBehaviorTest cleanup

e15ec20

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

4236bc4

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

dc7bb10

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - BrowserToolbarBehaviorTest cleanup

fcf8f38

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

Mugurell mentioned this issue

GeckoView - InputResultDetail syncronization issues #10585

Closed

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

553c4ed

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

fc5a79e

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - BrowserToolbarBehaviorTest cleanup

160ec02

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

9891d33

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

f9e0907

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

Mugurell added a commit to Mugurell/android-components that referenced this issue


          For mozilla-mobile#10555 - BrowserToolbarBehaviorTest cleanup

a6f47fd

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

mergify bot pushed a commit that referenced this issue


          For #10555 - Use INPUT_HANDLING_UNKNOWN as the default of InputResult…

c5e1ead

…Detail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

mergify bot pushed a commit that referenced this issue


          For #10555 - Fix InputResultDetail hashcode() collisions

261311e

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

mergify bot pushed a commit that referenced this issue


          For #10555 - BrowserToolbarBehaviorTest cleanup

01ca80d

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

grigoryk pushed a commit to gabrielluong/android-components that referenced this issue


          For mozilla-mobile#10555 - Use INPUT_HANDLING_UNKNOWN as the default …

8676bd6

…of InputResultDetail

We need to wait until having a response from GeckoView on how it handled the
touch only after which we'll know whether to animate the toolbar or not.

The edgecase scenario of having pull to refresh enabled even before having a
response from GeckoView will still work because "canOverscrollTop()" only
checks for the touch to not be handled by the browser to pan the page.

grigoryk pushed a commit to gabrielluong/android-components that referenced this issue


          For mozilla-mobile#10555 - Fix InputResultDetail hashcode() collisions

04c6dda

The previous implementation was affected by the fact that the class has 3
integer properties, all with values [0..4] (-1 was added recently) and it
would just add those value to compute the hashcode.
Collisions were a given.

By using decimal places for each property the new implementation should avoid
collisions while allowing for all the other expected guarantees.

grigoryk pushed a commit to gabrielluong/android-components that referenced this issue


          For mozilla-mobile#10555 - BrowserToolbarBehaviorTest cleanup

c836793

This commented method seems to be a leftover from the previous refactoring.
"behavior.forceExpand(..)" now calls "expandWithAnimation" for which we already
have a test at line 450.

Contributor Author

Mugurell commented Sep 14, 2021

Verified as fixed in Fenix - mozilla-mobile/fenix#16352 (comment)

Mugurell closed this as completed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.