Floating toolbar does not hide when the selection highlight is scrolled out of viewport

VERIFIED FIXED in Firefox 50

Status

()

defect
P1
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: TYLin, Assigned: TYLin)

Tracking

({regression})

unspecified
Firefox 52
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 unaffected, fennec50+, firefox50 verified, firefox51 verified, firefox52 verified)

Details

Attachments

(2 attachments)

Steps to reproduce:
1. Open an arbitrary web page like https://en.m.wikipedia.org/wiki/Main_Page#
2. Long tap to make a selection.
3. Scroll the entire selection up until it's out of viewport.

Expected result:
The floating toolbar hides.

Actual result:
The floating toolbar still shows.
I can reproduce this on nightly 52 and aurora 51, but not on release 49.
Keywords: regression
mozregression finds bug 1266369.
Blocks: 1266369
Flags: needinfo?(s.kaspari)
We are adding a constant value (20dp) to the content rect to make it not overlap the handles of the text selection. However if the rect is scrolled out of the screen then it has a height of 0 and that's what we detect in order to hide the toolbar. After the patch from bug 1266369 the rect is always at least 20dp and we never hide the toolbar. I guess a simple fix would be to just add the 20dp if the height is > 0.
tracking-fennec: --- → ?
OS: Unspecified → Android
Hardware: Unspecified → All
The solution in comment 3 is easy to fix :)
Assignee: nobody → tlin
Flags: needinfo?(s.kaspari)
Comment on attachment 8797902 [details]
Bug 1307100 - Add handlesOffset only if height > 0.

https://reviewboard.mozilla.org/r/83494/#review82152

LGTM - assuming you tested this? :)
Attachment #8797902 - Flags: review?(s.kaspari) → review+
Comment on attachment 8797902 [details]
Bug 1307100 - Add handlesOffset only if height > 0.

https://reviewboard.mozilla.org/r/83494/#review82152

Yes. I've tested this, and it works.
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dbaae495b5bf
Add handlesOffset only if height > 0. r=sebastian
tracking-fennec: ? → 50+
https://hg.mozilla.org/mozilla-central/rev/dbaae495b5bf
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Priority: -- → P1
Comment on attachment 8797902 [details]
Bug 1307100 - Add handlesOffset only if height > 0.

Approval Request Comment
[Feature/regressing bug #]: Bug 1266369. 
[User impact if declined]: Floating toolbar won't be hide when the selection highlight is scrolled out of viewport.
[Describe test coverage new/current, TreeHerder]: Merge on master, and test manually on my aurora build.
[Risks and why]: Low. Simple one line change.
[String/UUID change made/needed]: None.
Attachment #8797902 - Flags: approval-mozilla-aurora?
The original patch cannot be applied cleanly on beta.  I'll post the adjusted patch after manually testing on beta.
Comment on attachment 8799636 [details] [diff] [review]
Add handlesOffset only if height > 0. (for beta uplift)

Approval Request Comment
[Feature/regressing bug #]: Bug 1266369. 
[User impact if declined]: Floating toolbar won't be hide when the selection highlight is scrolled out of viewport.
[Describe test coverage new/current, TreeHerder]: Merge on master, and test manually on my beta build.
[Risks and why]: Low. Simple one line change.
[String/UUID change made/needed]: None.
Attachment #8799636 - Flags: approval-mozilla-beta?
Hi Ting-Yu, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(tlin)
Comment on attachment 8799636 [details] [diff] [review]
Add handlesOffset only if height > 0. (for beta uplift)

Fixes a new regression in 50, Beta50+
Attachment #8799636 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8797902 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in Nightly 52.0a1 (2016-10-11) on Sony Z3 Compact (Android 6.0.1).
Status: RESOLVED → VERIFIED
Flags: needinfo?(tlin)
Verified as fixed on both Aurora(51.0a2 / 2016-10-18) and Beta (50.0b8) on a Samsung Galaxy S6 Edge(Android 6.0.
You need to log in before you can comment on or make changes to this bug.