Closed Bug 1475325 Opened Last year Closed Last year

GeckoView: Floating text selection toolbar does not get dismissed if GeckoView gets destroyed

Categories

(GeckoView :: General, defect, P2)

All
Android
defect

Tracking

(geckoview62 fixed, firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 fixed, firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
geckoview62 --- fixed
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: sebastian, Assigned: jchen)

Details

(Whiteboard: [geckoview:klar:p2])

Attachments

(1 file, 1 obsolete file)

STR in a Focus build with GV:
* Load a website with text (It needs to be the first page you load! No history!)
* Select text
* Press back

Expected:
The tab gets destroyed, the home screen is displayed and the text selection toolbar is gone.

Actual:
The tab gets destroyed, the home screen is displayed and the floating text selection toolbar is still visible.

Note that it works fine if there's history and we go back in history.

Focus issue: https://github.com/mozilla-mobile/focus-android/issues/2633
P2 because this sounds like an uncommon scenario since this only happens on the first page and then the user has to press back to the home screen. The Focus issue is priority P3.
Priority: -- → P2
Whiteboard: [geckoview:klar] → [geckoview:klar:p2]
Assignee: nobody → mbrubeck
I can't reproduce this in the current GeckoView Focus nightly from the Play store (6.3 build #222071208 gecko 62.0.20180712042337), testing on a Pixel and a Pixel 2 XL, both running Android 8.1.0.  Can you still reproduce this?
Flags: needinfo?(s.kaspari)
I can't reproduce this in Focus Nightly anymore.
Status: NEW → RESOLVED
Closed: Last year
Flags: needinfo?(s.kaspari)
Resolution: --- → WORKSFORME
We're seeing this behavior again in Nightly off the 62 Rel Branch. https://github.com/mozilla-mobile/focus-android/issues/3317
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Priority: P2 → --
Consensus in GV triage is that this should be a Focus fix.
Assignee: mbrubeck → nobody
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → INVALID
After talking to Emily, I think this is something we can address in GV.
Assignee: nobody → nchen
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Hide the selection action toolbar when GeckoView is blurred so the we
don't end up with leftover visible toolbars.
Jim, should we uplift this fix to GV 62?

Even if this fix doesn't make next Tuesday's Focus 7.0 release, we will want to continue fixing bugs in the GV 62 relbranch for future Focus releases (until they can update to GV 63).
Flags: needinfo?(nchen)
Priority: -- → P2
Comment on attachment 9008904 [details]
Bug 1475325 - Hide selection action in some scenarios; r?droeh

Dylan Roeh (:droeh) has approved the revision.
Attachment #9008904 - Flags: review+
Yeah we should uplift
Flags: needinfo?(nchen)
Thanks Jim, can you request approval-mozilla-geckoview62 on the patch? (I _think_ that's the process here)
Flags: needinfo?(nchen)
Still working on the patch but yes I will request the flag.
Flags: needinfo?(nchen)
Attachment #9008904 - Attachment description: Bug 1475325 - Hide selection action when view is blurred; r?droeh → Bug 1475325 - Hide selection action in some scenarios; r?droeh
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/14ca7a3794fa
Hide selection action in some scenarios; r=droeh
https://hg.mozilla.org/mozilla-central/rev/14ca7a3794fa
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Comment on attachment 9008904 [details]
Bug 1475325 - Hide selection action in some scenarios; r?droeh

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: The text selection UI can stay on the screen for longer than it should, causing annoyance.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Small patch that only tries to dismiss the text selection UI when necessary.
[String changes made/needed]: None
Attachment #9008904 - Flags: approval-mozilla-beta?
Comment on attachment 9008904 [details]
Bug 1475325 - Hide selection action in some scenarios; r?droeh

[Approval Request Comment]

If this is not a sec:{high,crit} bug, please state case for consideration: This causes a bug in Focus that's based on GV 62

User impact if declined: Text selection UI can stay on the screen for longer than it should, causing an annoyance

Fix Landed on Version: 64

Risk to taking this patch (and alternatives if risky): Very small; the patch only tries to dismiss the selection UI when necessary

String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/Uplift_rules for more info.
Attachment #9008904 - Flags: approval-mozilla-geckoview62?
Add some tests for hiding selection action during navigation, blurring,
or clearing delegate.
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef88efc93b44
2. Add tests for hiding selection action in different cases; r=jchen
Comment on attachment 9008904 [details]
Bug 1475325 - Hide selection action in some scenarios; r?droeh

low-risk, Beta63+, GV62+
Attachment #9008904 - Flags: approval-mozilla-geckoview62?
Attachment #9008904 - Flags: approval-mozilla-geckoview62+
Attachment #9008904 - Flags: approval-mozilla-beta?
Attachment #9008904 - Flags: approval-mozilla-beta+
Backed out changeset f6a4fa55f724 (bug 1475325) for failures in org.mozilla.geckoview.test.SelectionActionDelegateTest.clear. a=backout

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=200365764&repo=mozilla-beta&lineNumber=3315

INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=clearDelegate[#text]
[task 2018-09-20T05:43:10.177Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2018-09-20T05:43:10.177Z] 05:43:10  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.SelectionActionDelegateTest.clearDelegate[#text] | status -2
[task 2018-09-20T05:43:10.178Z] 05:43:10     INFO -  TEST-INFO took 1010ms
[task 2018-09-20T05:43:10.245Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2018-09-20T05:43:10.246Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=118
[task 2018-09-20T05:43:10.247Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.SelectionActionDelegateTest
[task 2018-09-20T05:43:10.247Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2018-09-20T05:43:10.248Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=187
[task 2018-09-20T05:43:10.249Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=collapseToEnd[#input]
[task 2018-09-20T05:43:10.250Z] 05:43:10     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: 1
[task 2018-09-20T05:43:10.250Z] 05:43:10     INFO -  TEST-START | org.mozilla.geckoview.test.SelectionActionDelegateTest.collapseToEnd[#input]
[task 2018-09-20T05:43:13.453Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2018-09-20T05:43:13.453Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=118
[task 2018-09-20T05:43:13.454Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.SelectionActionDelegateTest
[task 2018-09-20T05:43:13.454Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=.
[task 2018-09-20T05:43:13.454Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=187
[task 2018-09-20T05:43:13.455Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=collapseToEnd[#input]
[task 2018-09-20T05:43:13.455Z] 05:43:13     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: 0
[task 2018-09-20T05:43:13.456Z] 05:43:13     INFO -  TEST-PASS | org.mozilla.geckoview.test.SelectionActionDelegateTest.collapseToEnd[#input] | took 3208ms

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&group_state=expanded&revision=f6a4fa55f7245e3860503595c74d5b24e7ff0642

Backout:
https://hg.mozilla.org/releases/mozilla-beta/rev/82ac63880ff38d905df3693ae0c3f604ed5d3307
Flags: needinfo?(nchen)
Bogdan, could you uplift both of these commits to Beta again? Thanks!

https://hg.mozilla.org/mozilla-central/rev/14ca7a3794fa
https://hg.mozilla.org/mozilla-central/rev/ef88efc93b44
Flags: needinfo?(nchen) → needinfo?(btara)
Attachment #9010039 - Attachment is obsolete: true
status-geckoview62=affedcted because Focus product management would like to get this bug fixed in Focus 7.0 (before October 2).
(In reply to Jim Chen [:jchen] [:darchons] from comment #23)
> Bogdan, could you uplift both of these commits to Beta again? Thanks!
> 
> https://hg.mozilla.org/mozilla-central/rev/14ca7a3794fa
> https://hg.mozilla.org/mozilla-central/rev/ef88efc93b44

i get the following when trying to uplift: 


mozilla@ubuntu:~/mozilla-unified$ hg graft -er 14ca7a3794fa::ef88efc93b44
skipping ungraftable merge revision 497639
skipping ungraftable merge revision 497668
skipping ungraftable merge revision 497698
skipping ungraftable merge revision 497823
skipping revision 497691:d073607e0ffa (already grafted to 497903:10bb8728016a)
skipping revision 497850:52871b889095 (already grafted to 497904:9e7e697907a4)
skipping revision 497877:ef88efc93b44 (already grafted to 497911:f6a4fa55f724)
skipping revision 497857:10f0ecb6ed63 (already grafted to 497913:9cfdcb17b563)
skipping revision 497673:8320750a187e (already grafted to 497931:f139ab75d0c9)
grafting 497629:14ca7a3794fa "Bug 1475325 - Hide selection action in some scenar                                                                                                                                                             ios; r=droeh"
merging mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSessio                                                                                                                                                             n.java
grafting 497640:59759503b889 "Bug 1483533 - Delay texture delete for DirectMapTe                                                                                                                                                             xtureSource r=jrmuizel"
merging gfx/layers/composite/TextureHost.cpp
grafting 497641:131a80cf6661 "Bug 1491289 - re-vendor libprio to pick up fixes f                                                                                                                                                             or using system NSS r=glandium"
grafting 497642:e4990387dd8c "Bug 1491639 - rename function IS_TABLE_CELL r=dbaron"
merging dom/html/nsGenericHTMLElement.cpp
merging layout/base/nsCSSFrameConstructor.cpp
merging layout/generic/ReflowInput.cpp
merging layout/mathml/nsMathMLmtableFrame.cpp
merging layout/tables/nsTableFrame.cpp
warning: conflicts while merging layout/generic/ReflowInput.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
mozilla@ubuntu:~/mozilla-unified$
Flags: needinfo?(btara) → needinfo?(nchen)
OK I can take care of it. Thanks!
Flags: needinfo?(nchen)
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 64 → mozilla64
You need to log in before you can comment on or make changes to this bug.