Closed Bug 1581161 Opened 3 months ago Closed 25 days ago

Improve SelectionActionDelegate API


(GeckoView :: General, enhancement, P2)



(firefox72 fixed)

Tracking Status
firefox72 --- fixed


(Reporter: snorp, Assigned: gl)




(1 file, 1 obsolete file)

Currently, SelectionActionDelegate API is a little clumsy wrt to asking Gecko to perform actions. You're supposed to use the GeckoResponse<String> passed with onShowActionRequest for this, but it's not (to me) very obvious how it's used. It would be more clear to just add a perform() method to the Selection object. We could move the ACTION_FOO constants there as well (and make them integers while we're at it, similar to all the other similar types we have).

Assignee: nobody → estirling

Thinking further, we can probably just kill all of the ACTION_FOO constants in lieu of methods on Selection. This is a lot more discoverable, and we could also have arguments if necessary (like if you want to ask Gecko to move the selection handles).

work in progress

Priority: -- → P2

In bug 1570810 comment 7, Sebastian offers some suggestions for improving SelectionActionDelegate:

In our case we do not want to change any of the implementation and reuse BasicSelectionActionDelegate. We need to know if a text selection toolbar is showing (internally mResponse != null) and have a way to clear it (clearSelection()).

(1) One option would be to create those methods or make them accessible so that we can call them by going through GeckoSession and the delegate. Currently clearSelection() is protected and mResponse is not visible.

(2) As another option we could create our own SelectionActionDelegate implementation that delegates to a BasicSelectionActionDelegate object (which basically means implementing option 1 ourselves). But that is quite hard: BasicSelectionActionDelegate needs an Activity that doesn't always exist since it has a different lifecycle. Internally GeckoView seems to do some dance to set and clear the delegate depending on whether a session is rendered or released. Replicating this seems quite complicated and error-prone.

Attachment #9105923 - Attachment is obsolete: true
Assignee: estirling → gl
Pushed by
Removed GeckoResponse and reworked Text Selection API. r=geckoview-reviewers,snorp,esawin
Closed: 25 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.