Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-17754] Throw an event from webview #6528

Merged
merged 3 commits into from Jan 8, 2015

Conversation

ashcoding
Copy link
Contributor

@ashcoding
Copy link
Contributor Author

Test code. Replace link with the ajax site you want to test with.
onLoadResource is available on all Android API level.

shouldInterceptRequest is only available on API level 11 aka Honeycomb devices. If used on Gingerbread (level 10), it won't be available.

var win = Ti.UI.createWindow();
//Please put a link with the ajax to test this
var webview = Titanium.UI.createWebView({url:'http://ajaxtoolbox.com/request/examples.php'});

webview.addEventListener('onLoadResource', function(e) {
Ti.API.info("onLoadResource Event Listener "+ e.url);
});

webview.addEventListener('shouldInterceptRequest', function(e) {
Ti.API.info("shouldInterceptRequest Event Listener " + e.url );
});

win.add(webview);
win.open();

@Override
public WebResourceResponse shouldInterceptRequest(WebView view, String url)
{
if(Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'shouldInterceptRequest' event seems to return the same thing as 'onloadResource' event. Why do we need to fire 2 events back to back if they give the same information? Perhaps I'm missing something here? Also 'shouldInterceptRequest' event is deprecated in Lollipop, so I'm not sure we should add a feature that is already deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's actually 2 versions of 'shouldInterceptRequest'.

'WebResourceResponse shouldInterceptRequest(WebView view, String url)' is deprecated from Lollipop.
'WebResourceResponse shouldInterceptRequest(WebView view, WebResourceRequest request)' is the new API available from Lollipop onwards to replace the deprecated one.

The 2 events fire on slightly different occasions but, at the end, for the user, it might be the same.
'shouldInterceptRequest' -> Notify the host application of a resource request
'onLoadResource' -> Notify the host application that the WebView will load the resource
Ref: http://developer.android.com/reference/android/webkit/WebViewClient.html

Thus, for this use case, would it be better to remove 'shouldInterceptRequest' and only use 'onLoadResource'? 'onLoadResource' would be cleaner too as it is available from API level 1. If so, I'll remove 'shouldInterceptRequest'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 'onLoadResource' event is satisfactory. We shouldn't add any feature that is already deprecated.

@hieupham007
Copy link
Contributor

Code reviewed and functionally tested. Please address comment

super.onLoadResource(view, url);
KrollDict data = new KrollDict();
data.put(TiC.PROPERTY_URL, url);
webView.getProxy().fireSyncEvent(TiC.EVENT_WEBVIEW_ON_LOAD_RESOURCE, data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on why this event needs to be synchronous?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't have to be synchronous. I'll change this.

@hieupham007
Copy link
Contributor

Looks good functional, just a few minor comments

@ashcoding ashcoding force-pushed the timob-17754 branch 2 times, most recently from 4b07cd0 to d44430b Compare January 8, 2015 02:54
@hieupham007
Copy link
Contributor

Looks good, merging

hieupham007 added a commit that referenced this pull request Jan 8, 2015
[TIMOB-17754] Throw an event from webview
@hieupham007 hieupham007 merged commit cd184e8 into tidev:master Jan 8, 2015
@ashcoding ashcoding deleted the timob-17754 branch May 21, 2015 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants