1052841 - Don't extend GeckoEventListener in GeckoView

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Attachment: Remove handleMessage from GeckoView

If we don't want handleMessage in the public API, we should remove it.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8471883 [details] [diff] [review]
Remove handleMessage from GeckoView

LGTM

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

This will likely affect bug 1042257

Comment 3

[Nick Alexander :nalexander [he/him]]

Is the idea that GeckoView consumers who want to do "interesting" things with GeckoView and Gecko JavaScript execution will register their own EventListeners?  If so, I think this is a good idea.

Comment 4

[Marco Zehe (:MarcoZ)]

Does this negatively impact our interaction with TalkBack or AccessFu?

Comment 5

[Brian Nicholson (:bnicholson)]

(In reply to Nick Alexander :nalexander from comment #3)
> Is the idea that GeckoView consumers who want to do "interesting" things
> with GeckoView and Gecko JavaScript execution will register their own
> EventListeners?  If so, I think this is a good idea.

Do we intend to expose our event API GeckoView consumers at all? I'm glad this is coming up again, because I wasn't sure when I wrote this response yesterday: https://mail.mozilla.org/pipermail/mobile-firefox-dev/2014-August/000848.html

Looking at the existing GeckoView API, I was assuming that we intended to encapsulate all Gecko communication through the GeckoView/GeckoInterface API, and I couldn't find any docs that said otherwise. I'll correct myself on the mailing list if that's not the case.

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Brian Nicholson (:bnicholson) from comment #5)
> (In reply to Nick Alexander :nalexander from comment #3)
> > Is the idea that GeckoView consumers who want to do "interesting" things
> > with GeckoView and Gecko JavaScript execution will register their own
> > EventListeners?  If so, I think this is a good idea.
> 
> Do we intend to expose our event API GeckoView consumers at all? I'm glad
> this is coming up again, because I wasn't sure when I wrote this response
> yesterday:
> https://mail.mozilla.org/pipermail/mobile-firefox-dev/2014-August/000848.html
> 
> Looking at the existing GeckoView API, I was assuming that we intended to
> encapsulate all Gecko communication through the GeckoView/GeckoInterface
> API, and I couldn't find any docs that said otherwise. I'll correct myself
> on the mailing list if that's not the case.

I wasn't clear either.  I starred your email but don't really know what I think yet.  My initial reaction was, "why are we doing this"?  I have a hard time believing a consumer can build a web browser around GeckoView without doing some event handling.  If that's the case, then we shouldn't be making this harder.

But maybe building a *browser* around GeckoView is not the motivation?  Maybe it's more about substituting for a (Chrome) WebView?

Comment 7

[Brian Nicholson (:bnicholson)]

I know that one of our pipe dreams was to have Fennec eventually be a GeckoView consumer itself, so unless we've decided against that, I guess we'll be required to expose all the dirty stuff as you said.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

We *might* create a public wrapper to allow clients to use some form of message passing to communicate between Java and Web Content, but we don't know what that will look like and we probably want to abstract/insulate the details of our message passing.

Comment 9

[Frédéric Wang (:fredw)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> This will likely affect bug 1042257

As I see, I'll only need to move the handlers for accessibility message in private functions. I plan to update & split the patches for bug 1042257 anyway. But I'm not really sure whether you decided or not to take attachment 8471883 [details] [diff] [review]?

Comment 10

[Brian Nicholson (:bnicholson)]

(In reply to Frédéric Wang (:fredw) from comment #9)
> (In reply to Mark Finkle (:mfinkle) from comment #2)
> > This will likely affect bug 1042257
> 
> As I see, I'll only need to move the handlers for accessibility message in
> private functions. I plan to update & split the patches for bug 1042257
> anyway.

Yeah, the bitrot for bug 1042257 should be pretty minor. Since you're adding a handleMessage there for NativeJSObject, I'd recommend doing the same thing: make the listener private instead of attaching a public method on GeckoView with an "ignore me" comment.

> But I'm not really sure whether you decided or not to take
> attachment 8471883 [details] [diff] [review]?

I think we want it -- we should encapsulate these listeners no matter what the future GeckoView API looks like.

https://hg.mozilla.org/integration/fx-team/rev/070d5351ed82

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/070d5351ed82

Comment 12

[Frédéric Wang (:fredw)]

(In reply to Brian Nicholson (:bnicholson) from comment #10)
> Yeah, the bitrot for bug 1042257 should be pretty minor. Since you're adding
> a handleMessage there for NativeJSObject, I'd recommend doing the same
> thing: make the listener private instead of attaching a public method on
> GeckoView with an "ignore me" comment.

Yes, that's what I intended to do ;-)

> 
> > But I'm not really sure whether you decided or not to take
> > attachment 8471883 [details] [diff] [review]?
> 
> I think we want it -- we should encapsulate these listeners no matter what
> the future GeckoView API looks like.
> 
> https://hg.mozilla.org/integration/fx-team/rev/070d5351ed82

OK, thanks for the info.

Comment 13

[Frédéric Wang (:fredw)]

(In reply to Marco Zehe (:MarcoZ) from comment #4)
> Does this negatively impact our interaction with TalkBack or AccessFu?

I expect that it does not. It's really just a matter of deciding whether or not we expose these handleMessage functions. Mark's comment was only to warn about bitrot issues.