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-25462] Android: Implement Ti.UI.WebView.userAgent #9576

Merged
merged 7 commits into from Nov 13, 2017

Conversation

garymathews
Copy link
Contributor

  • Implement userAgent property for parity with iOS
TEST CASE
var win = Ti.UI.createWindow({backgroundColor: 'gray'}),
    webView = Ti.UI.createWebView({
        url: 'http://whatsmyuseragent.org',
        userAgent: 'TEST AGENT'
    });

webView.addEventListener('load', function(e) {
    var exp = /user-agent.+\s+.+\>(.*)\</g.exec(e.source.html),
        userAgent = exp && exp.length > 1 ? exp[1] : undefined;

    alert(userAgent);
});

win.add(webView);
win.open();

JIRA Ticket

@garymathews garymathews added this to the 7.0.0 milestone Oct 31, 2017
@hansemannn hansemannn changed the title [TIMOB-25462] Android: Implement Ti.US.WebView.userAgent [TIMOB-25462] Android: Implement Ti.UI.WebView.userAgent Oct 31, 2017
@@ -460,6 +460,10 @@ public void processProperties(KrollDict d)
if (d.containsKey(TiC.PROPERTY_DISABLE_CONTEXT_MENU)) {
disableContextMenu = TiConvert.toBoolean(d, TiC.PROPERTY_DISABLE_CONTEXT_MENU);
}

if (d.containsKey(TiC.PROPERTY_USER_AGENT)) {
Copy link
Contributor

@ypbnv ypbnv Nov 1, 2017

Choose a reason for hiding this comment

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

processProperties is called after the creation of the native WebView and this causes a behavior that is a bit confusing - the WebView fires 'load' event twice. This seems to be an issue in chromium itself:
https://bugs.chromium.org/p/chromium/issues/detail?id=315891
Changing the UserAgent while loading a page provokes a reload. I could not find more recent discussion about that.
The result is that the test may fail because of that. The first 'load' event is delivered with an empty html and the UserAgent will never match the value from the creation dictionary.

Setting a userAgent value from the creation dictionary when preparing the WebView settings before initializing it seems to avoid this:
https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java#L301

Something like that maybe:

if (proxy.hasPropertyAndNotNull(TiC.PROPERTY_USER_AGENT)) {
    settings.setUserAgentString(TiConvert.toString(proxy.getProperty(TiC.PROPERTY_USER_AGENT)));
}

Still that leaves us with that undesired result when changing the UserAgent after the WebView has been created. I am not sure what is the behavior on iOS side:
http://docs.appcelerator.com/platform/latest/#!/api/Titanium.UI.WebView-property-userAgent
But does it make sense to set the property as Creation Only ?

CC: @hansemannn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ypbnv nice catch! I'll update the PR to use the creation dictionary 👍

it('userAgent', function (finish) {
win = Ti.UI.createWindow({backgroundColor: 'gray'});
var webView = Ti.UI.createWebView({
url: 'http://whatsmyuseragent.org',
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is with spaces for some of the next lines. Using tabs would match the rest of the file.

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Nov 7, 2017

@garymathews , when I run the test, I see 2 alerts one with no value before the webview loads & the other with the value TEST AGENT after it loads.

@garymathews
Copy link
Contributor Author

@lokeshchdhry Updated PR

@lokeshchdhry
Copy link
Contributor

FR Passed.

User Agent is set & detected successfully.

Studio Ver: 4.10.0.201709271713
SDK Ver: 7.0.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.10
Appc CLI: 6.3.0
Ti CLI Ver: 5.0.14
Alloy Ver: 1.10.7
Node Ver: 7.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 6P --- Android 8.0.0
⇨ google Nexus 5 --- Android 6.0.1

@build build added the android label Nov 13, 2017
@eric34 eric34 merged commit 43395bb into tidev:master Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants