shouldLoadURL event support and executeAndReturnString()

[codenameone]

Original issue 459 created by codenameone on 2012-12-31T21:45:51.000Z:

This is related to issue # 453 (http://code.google.com/p/codenameone/issues/detail?id=453). I have implemented two mechanisms necessary to support a robust Javascript interface in a minimal way. These are:

1. A shouldLoadURL event that is called whenever the browser is about to try to load a URL. It gives the developer the ability to cancel the page load, and/or handle the URL processing itself. With respect to Javascript, this allows us to design our own custom URLs that Javascript calls via window.location to send messages back into Java.
2. An executeAndReturnString() method on the BrowserComponent class that executes a javascript string and returns the string result of the processing. This method is synchronous so it makes it much more convenient to query the javascript environment and DOM from Java.

I have added support for Android, iOS, and JavaSE, and have attached the patches for these ports all separately.

I have also created a Test application to test all of this functionality (and will hopefully grow to include tests for much more CodenameOne functionality). I have added this project to the incubator:
http://code.google.com/p/codenameone-incubator/source/browse/#svn%2Ftrunk%2Fshannah%2FCodenameOneTests

However I need to clean it up to make it easier for others to set it up and use it. This project includes a separate package, ca.weblite.codename1.js that is a full two-way Javascript bridge built on top of this functionality.

http://code.google.com/p/codenameone-incubator/source/browse/#svn%2Ftrunk%2Fshannah%2FCodenameOneTests%2Fsrc%2Fca%2Fweblite%2Fcodename1%2Fjs

I will be factoring this out into its own subproject and will post usage examples and more tests later today.

I have tested these changes on:
iPhone 4S
Android (Nexus 7)
Android (HTC Magic running Android 2.1)
JavaSE (the simulator)

I don't have a Blackberry test environment up and running yet, or I would have also provided a patch for it also.

[codenameone]

Comment #1 originally posted by codenameone on 2013-01-01T06:13:09.000Z:

Issue 453 has been merged into this issue.

[codenameone]

Comment #2 originally posted by codenameone on 2013-01-01T06:13:33.000Z:

<empty>

[codenameone]

Comment #3 originally posted by codenameone on 2013-01-01T07:28:45.000Z:

Thanks for the patch. I like the general approach of the patch but there are a couple of things that I think might be confusing to users of this functionality.

The first and probably worst one is the event dispatch off the EDT, this breaks a major rule we always made in Codename One where all events are sent on the EDT. I'd rather have no event.
The second issue is the usage of the should method which then requires the WebComponent to subclass BrowserComponent just to implement that method.

I think both can be resolved by adding an interface to the events package called: BrowserNavigationCallback, then add methods to both classes called setBrowserNavigationCallback(). The advantage here is a cleaner separation (no need to subclass) and since the interface will not use the listener API we can much more clearly document the not on the EDT behavior in the interface and in the setter method on the components.
This will also simplify the usage in comparison to the current requirement to consume an event in order to indicate navigation which isn't intuitive.

Does that make sense?

[codenameone]

Comment #4 originally posted by codenameone on 2013-01-01T07:57:17.000Z:

I think I understand the idea, but I'm not entirely clear yet on the
details of the changes. Would the BrowserNavigationCallback interface
include a method like shouldLoadURL, or would it provide its own
event handling mechanism (e.g. addNavigationCallback) so that multiple
event handlers could be added to it for potentially many different
types of navigation callback events?

I know that most of the native toolkits use the delegate pattern for
this type of callback, rather than events. However events can be
handy for allowing multiple objects to subscribe to receive and
respond to these events.

-Steve

[codenameone]

Comment #5 originally posted by codenameone on 2013-01-01T08:00:27.000Z:

It should just be an interface declaring the should navigate method and returning a boolean value.
Events can be handy but since this is a blocking operation and we want it to be VERY fast I think events might be an overkill. It would be trivial for someone who is interested in a publish/subscribe pattern to create an implementation that delivers the callback as an event if such a use case arises.
I'd also like to keep everything as simple as possible since we are already in code freeze but I really want this change to make it to 1.0.

[codenameone]

Comment #6 originally posted by codenameone on 2013-01-01T08:07:48.000Z:

OK. I see what you mean. I'll make the change in the morning and
resubmit the patch.

-Steve

[codenameone]

Comment #7 originally posted by codenameone on 2013-01-01T08:11:39.000Z:

Which package would you like the BrowserNavigationCallback interface added to?

[codenameone]

Comment #8 originally posted by codenameone on 2013-01-01T08:12:46.000Z:

Sorry... I missed it. You already said it should be added to the events package.

-steve

[codenameone]

Comment #9 originally posted by codenameone on 2013-01-01T08:13:29.000Z:

My initial thought is the events package mostly because I can't think of anything else.

[codenameone]

Comment #10 originally posted by codenameone on 2013-01-01T09:51:57.000Z:

I have made the changes discussed and have included all of the changes in a single patch file. I have tested the changes on the Simulator, Android (Nexus 7), and iPhone 4s and they seem to work. Need to add javadocs and other documentation still, but wanted to get this in before I quit for the night.

[codenameone]

Comment #11 originally posted by codenameone on 2013-01-01T12:06:22.000Z:

The JavascriptInterface annotation is problematic since it might fail on 2.x devices and can't compile with our current 14 level API deployment.

[codenameone]

Comment #12 originally posted by codenameone on 2013-01-01T12:39:38.000Z:

I committed most of this and added the docs. However, I didn't commit the Android version because of the concerns of the annotations issues. Thanks.

[codenameone]

Comment #13 originally posted by codenameone on 2013-01-01T13:40:45.000Z:

I was concerned about this too but I tested it on a 2.1 device (level 7)
and it worked. I was going to attempt some misdirection using a separate
file and Class.forName but decided not to when the code worked. I expected
it to fail and am a little surprised that it didn't. Perhaps it is because
it is an annotation and doesn't actually involve any runtime code so it is
just ignored on the older version?

[codenameone]

Comment #14 originally posted by codenameone on 2013-01-01T13:44:08.000Z:

The annotation has to include runtime code otherwise it wouldn't work. If there is no other way to do this I will try to integrate it this way.

[codenameone]

Comment #15 originally posted by codenameone on 2013-01-01T14:00:47.000Z:

Perhaps this explains it.
http://stackoverflow.com/questions/3567413/why-doesnt-a-missing-annotation-cause-a-classnotfoundexception-at-runtime

Evidently an annotation doesn't need to be present at runtime and will just
be ignored if its binary class cannot be found.

Apparently this annotation is only necessary in 4.x but I don't have any
test devices running that new of version (nexus 7 runs 3.x) so it makes no
difference on my test devices whether I use it if not. I figure its best
to try and be compatible with the latest though.

Steve

[codenameone]

Comment #16 originally posted by codenameone on 2013-01-01T14:02:15.000Z:

Doesn't the nexus 7 run Jelly Bean?

[codenameone]

Comment #17 originally posted by codenameone on 2013-01-01T14:05:05.000Z:

Which API level are you compiling against?
In which API level was this declared? Their docs aren't very clear there.

[codenameone]

Comment #18 originally posted by codenameone on 2013-01-01T14:06:48.000Z:

Oops youre right. I was looking at the kernel version....

[codenameone]

Comment #19 originally posted by codenameone on 2013-01-01T14:07:54.000Z:

Found it, Level 17. I'm assigning to Chen to evaluate whether its practical to migrate to 17.

[codenameone]

Comment #20 originally posted by codenameone on 2013-01-01T14:14:19.000Z:

I'm targeting minsdk version 7.

I'm compiling using ADT Build: v21.0.1-543035 (think this means sdk
level 21). My Manifest file looks like:

<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION"/>
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION"/>

<uses-sdk android:minSdkVersion="7"/>

<application android:label="#APPLICATION_NAME#">
    <activity android:name="com.codename1.impl.android.AndroidStub"
              android:label="#APPLICATION_NAME#">
        <intent-filter>
            <action android:name="android.intent.action.MAIN" />
            <category android:name="android.intent.category.LAUNCHER" />
        </intent-filter>
    </activity>
</application>

The docs say:

"Caution: If you've set your targetSdkVersion to 17 or higher, you
must add the @JavascriptInterface annotation to any method that you
want available your web page code (the method must also be public). If
you do not provide the annotation, then the method will not accessible
by your web page when running on Android 4.2 or higher."

It also says that this class was added in API level 17.
http://developer.android.com/reference/android/webkit/JavascriptInterface.html

-Steve

[codenameone]

Comment #21 originally posted by codenameone on 2013-01-01T14:16:19.000Z:

No wait... I'm an idiot. My SDK version is 17. (but minsdk was set
to 7 in the application manifest).

-Steve

[codenameone]

Comment #22 originally posted by codenameone on 2013-01-01T14:16:57.000Z:

So if I understand this correctly: as long as we are on SDK level 14 we don't need this annotation at all. Once we upgrade we MUST have it right?
Can you test by compiling on SDK 14 with the annotation stuff commented out?
If this works then that's probably the best, we will leave the code commented out and then comment it in when we migrate to 17 or newer.

[codenameone]

Comment #23 originally posted by codenameone on 2013-01-01T14:19:36.000Z:

BTW looking here: http://www.korri.net/tutorials/android/communication-to-and-from-android-webview-with-js.html it seems there is another approach that doesn't include the expose in JavaScript stuff.

[codenameone]

Comment #24 originally posted by codenameone on 2013-01-01T14:45:21.000Z:

After implementing the js bridge using this method I think it might be
painful to use it at this low level.....

However I may try doing it by overriding the alert behavior as, apparently,
phonegap does it.

This article highlights some security aspects of JavaScript interfaces
http://labs.mwrinfosecurity.com/blog/2012/04/30/building-android-javajavascript-bridges/

And makes me think that I should have avoided it from the start.
Overriding the alert seems like a much safer approach in hindsight,

Steve

[codenameone]

Comment #25 originally posted by codenameone on 2013-01-01T15:22:28.000Z:

So where do we stand? Should I use the current code with annotations commented out?

[codenameone]

Comment #26 originally posted by codenameone on 2013-01-01T17:35:05.000Z:

Sorry... Fell asleep. I'm going to test it out over the next couple of
hours to see if that will work . I'll let you know what I find.

Steve

[codenameone]

Comment #27 originally posted by codenameone on 2013-01-01T18:19:10.000Z:

Ok. Yes you can just comment the annotations out and it will work. I
tested this on 4.2.1 (nexus7) and it worked fine. Only when I set minsdk
version to 17 in the application manifest did it give me problems.

Steve

On Tuesday, January 1, 2013, Steve Hannah wrote:

[codenameone]

Comment #28 originally posted by codenameone on 2013-01-01T19:02:53.000Z:

Thanks, I'll commit the fix soon. I'm pending on another commit that is going in right now (updating the plugin) so once that is done this will also land and I'll update the server.

[codenameone]

Comment #29 originally posted by codenameone on 2013-01-08T09:09:48.000Z:

<empty>