Convert text selection events to bundle events

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
GeckoView
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
Created attachment 8820828 [details] [diff] [review]
1. Add Object overrides for GeckoBundle (v1)

Add overrides for Object methods in GeckoBundle, and redirected them to
the corresponding methods in SimpleArrayMap. For equals, a custom
implementation is used because the equals implementation in
SimpleArrayMap had bugs across different support library versions.
Attachment #8820828 - Flags: review?(s.kaspari)
Created attachment 8820829 [details] [diff] [review]
2. Convert text selection events to bundle events (v1)

Convert the "TextSelection:*" events to use BundleEventListener /
GeckoBundle, in both ActionBarTextSelection.java and
FloatingToolbarTextSelection.java. UI events are used because the
listeners require the UI thread. The "TextSelection:Update" listeners
are removed because the event is no longer sent by ActionBarHandler.
Attachment #8820829 - Flags: review?(s.kaspari)
Comment on attachment 8820828 [details] [diff] [review]
1. Add Object overrides for GeckoBundle (v1)

Review of attachment 8820828 [details] [diff] [review]:
-----------------------------------------------------------------

I wonder if it makes sense to add a junit4 test for GeckoBundle? Should be pretty straight forward.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/GeckoBundle.java
@@ +663,5 @@
> +    }
> +
> +    @Override // Object
> +    public String toString() {
> +        return mMap.toString();

Ah, that's a good thing to have. I recently wrote this locally too, for debugging.

> public String toString() {
>     return "GeckoBundle[" + mMap.toString() + "]";
> }
Attachment #8820828 - Flags: review?(s.kaspari) → review+
Attachment #8820829 - Flags: review?(s.kaspari) → review+
Blocks: 1326101
(In reply to Sebastian Kaspari (:sebastian) from comment #3)
> Comment on attachment 8820828 [details] [diff] [review]
> 1. Add Object overrides for GeckoBundle (v1)
> 
> Review of attachment 8820828 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I wonder if it makes sense to add a junit4 test for GeckoBundle? Should be
> pretty straight forward.

Filed bug 1326101; would be a nice intern project.

Comment 5

10 months ago
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58e189739172
1. Add Object overrides for GeckoBundle; r=sebastian
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a43ca16d1c1
2. Convert text selection events to bundle events; r=sebastian

Comment 6

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/58e189739172
https://hg.mozilla.org/mozilla-central/rev/2a43ca16d1c1
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.