Improve SelectionActionDelegate API
Categories
(GeckoView :: General, enhancement, P2)
Tracking
(firefox72 fixed)
| Tracking | Status | |
|---|---|---|
| firefox72 | --- | fixed |
People
(Reporter: snorp, Assigned: gl)
References
Details
Attachments
(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).
|
||
Updated•3 months ago
|
|
Reporter | |
Comment 1•3 months ago
|
||
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).
|
||
Comment 3•3 months ago
|
||
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.
|
|
||
Comment 4•3 months ago
|
||
|
Assignee | |
Comment 5•Last month
|
||
iDifferential Revision: https://phabricator.services.mozilla.com/D47116
|
||
Updated•Last month
|
|
|
Assignee | |
Updated•Last month
|
Pushed by gluong@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c9324524d51 Removed GeckoResponse and reworked Text Selection API. r=geckoview-reviewers,snorp,esawin
|
||
Comment 7•25 days ago
|
||
| bugherder | ||
Description
•