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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-25462] Android: Implement Ti.UI.WebView.userAgent #9576

Merged
merged 7 commits into from
Nov 13, 2017
Merged

[TIMOB-25462] Android: Implement Ti.UI.WebView.userAgent #9576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90d1fb9
[TIMOB-25462] Implement Ti.UI.WebView.userAgent
Oct 31, 2017
5ae2e8c
[TIMOB-25462] Add test case
Oct 31, 2017
c043584
[TIMOB-25462] Fix test formatting
Nov 1, 2017
ff006d7
[TIMOB-25462] Prevent 'load' event firing twice
Nov 7, 2017
9187114
Merge branch 'master' into TIMOB-25462
Nov 9, 2017
a2ca862
Merge branch 'master' into TIMOB-25462
sgtcoolguy Nov 9, 2017
e21c584
Merge branch 'master' into TIMOB-25462
sgtcoolguy Nov 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class TiUIWebView extends TiUIView
private TiWebChromeClient chromeClient;
private boolean bindingCodeInjected = false;
private boolean isLocalHTML = false;
private boolean disableContextMenu = false;
private HashMap<String, String> extraHeaders = new HashMap<String, String>();

private static Enum<?> enumPluginStateOff;
Expand All @@ -74,7 +75,10 @@ public class TiUIWebView extends TiUIView
public static final int PLUGIN_STATE_ON = 1;
public static final int PLUGIN_STATE_ON_DEMAND = 2;

private boolean disableContextMenu = false;
// TIMOB-25462: minor 'hack' to prevent 'beforeload' and 'load' being
// called when the user-agent has been changed, this is a chromium bug
// https://bugs.chromium.org/p/chromium/issues/detail?id=315891
public boolean hasSetUserAgent = false;

private static enum reloadTypes
{
Expand Down Expand Up @@ -460,6 +464,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 •
edited

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 👍

((WebViewProxy) getProxy()).setUserAgent(d.getString(TiC.PROPERTY_USER_AGENT));
}
}

@Override
Expand Down Expand Up @@ -491,6 +499,8 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP
}
} else if (TiC.PROPERTY_DISABLE_CONTEXT_MENU.equals(key)) {
disableContextMenu = TiConvert.toBoolean(newValue);
} else if (TiC.PROPERTY_USER_AGENT.equals(key)) {
((WebViewProxy) getProxy()).setUserAgent(TiConvert.toString(newValue));
} else {
super.propertyChanged(key, oldValue, newValue, proxy);
}
Expand Down Expand Up @@ -828,6 +838,7 @@ public void setUserAgentString(String userAgentString)
{
WebView currWebView = getWebView();
if (currWebView != null) {
hasSetUserAgent = true;
currWebView.getSettings().setUserAgentString(userAgentString);
}
}
Expand Down
5 changes: 3 additions & 2 deletions android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiWebViewClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public void onPageFinished(WebView view, String url)
{
super.onPageFinished(view, url);
WebViewProxy proxy = (WebViewProxy) webView.getProxy();
if (proxy == null) {
if (proxy == null || webView.hasSetUserAgent) {
webView.hasSetUserAgent = false;
return;
}
webView.changeProxyUrl(url);
Expand Down Expand Up @@ -80,7 +81,7 @@ public void onPageStarted(WebView view, String url, Bitmap favicon)
{
super.onPageStarted(view, url, favicon);
WebViewProxy proxy = (WebViewProxy) webView.getProxy();
if (proxy == null) {
if (proxy == null || webView.hasSetUserAgent) {
return;
}
KrollDict data = new KrollDict();
Expand Down
5 changes: 5 additions & 0 deletions android/titanium/src/java/org/appcelerator/titanium/TiC.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3013,6 +3013,11 @@ public class TiC
*/
public static final String PROPERTY_USE_SPINNER = "useSpinner";

/**
* @module.api
*/
public static final String PROPERTY_USER_AGENT = "userAgent";

/**
* @module.api
*/
Expand Down