[TIMOB-23993] Android: Expose disableContextMenu in TiWebView #8494

frankieandone · 2016-10-10T20:28:57Z

JIRA: https://jira.appcelerator.org/browse/timob-23993

cloned off: https://jira.appcelerator.org/browse/TIMOB-16369. Disable context menu.

…ems properties in app.js

… timob-23993

sgtcoolguy · 2016-10-11T13:50:46Z

Relates to #8488

sgtcoolguy · 2016-10-11T13:53:24Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java

 import android.webkit.WebSettings;
 import android.webkit.WebView;

-@SuppressWarnings("deprecation")
-public class TiUIWebView extends TiUIView
+@SuppressWarnings("deprecation") public class TiUIWebView extends TiUIView


formatting still looks off from our standard. annotations should be on the preceding line, not inline as they appear to be throughout your changes.

I reformatted code using https://github.com/appcelerator/appc-standards/blob/master/languages/java/eclipse-format-android.xml. It put the annotations in the same line @sgtcoolguy

I'm not sure you've imported it and used it properly then. The settings include these:

<setting id="org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_method" value="insert"/> <setting id="org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_type" value="insert"/>

Yep. The file I imported have those two lines. As for how I imported them: Preferences->Editor->Code Style->Java then import eclipse xml and select. For reformatting, cmd + a then code -> reformat code. Still same result.

sgtcoolguy · 2016-10-11T13:54:12Z

android/modules/ui/src/java/ti/modules/titanium/ui/WebViewProxy.java

@@ -37,7 +37,8 @@
 	TiC.PROPERTY_WEBVIEW_IGNORE_SSL_ERROR,
 	TiC.PROPERTY_OVER_SCROLL_MODE,
 	TiC.PROPERTY_CACHE_MODE,
-	TiC.PROPERTY_LIGHT_TOUCH_ENABLED
+	TiC.PROPERTY_LIGHT_TOUCH_ENABLED,
+	TiC.PROPERTY_CONTEXT_MENU_ITEMS


I think this ticket is just about addressing the ability to disable the context menu on text selection. So we shouldn't need a new contextMenuItems property. But we should need a new disableContextMenu property.

sgtcoolguy · 2016-10-11T13:54:56Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java

@@ -69,35 +74,128 @@
 	public static final int PLUGIN_STATE_ON = 1;
 	public static final int PLUGIN_STATE_ON_DEMAND = 2;

-	private static enum reloadTypes {
+	private boolean disableTextSelection = false;


Looks like @hansemannn has just done disableContextMenu in iOS (and not disableTextSelection). May want to limit it to the single property like he did.

sgtcoolguy · 2016-10-11T13:55:30Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java

+		}
+
+		public ActionMode nullifiedActionMode() {
+			return new ActionMode() {


Extract out a class NullActionMode?

sgtcoolguy · 2016-10-11T13:55:42Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java

+			@Override public boolean onCreateActionMode(ActionMode mode, Menu menu)
+			{
+				mode.getMenuInflater().inflate(R.menu.ti_custom_menu, menu);
+				//				try {


Remove commented out and unused code.

sgtcoolguy · 2016-10-11T13:58:48Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/webview/TiUIWebView.java

-				setUrl((String) reloadData);
-			} else {
-				Log.d(TAG, "reloadMethod points to url but reloadData is null or of wrong type. Calling default", Log.DEBUG_MODE);
+			case DATA:


retain existing formatting if you're not changing anything here to avoid confusion on reviews, please.

sgtcoolguy · 2016-10-11T13:59:14Z

android/titanium/src/java/org/appcelerator/titanium/TiC.java

@@ -1119,6 +1119,11 @@
 	/**
 	 * @module.api
 	 */
+	public static final String PROPERTY_CONTEXT_MENU_ITEMS = "contextMenuItems";


sgtcoolguy · 2016-10-11T14:00:14Z

android/titanium/res/menu/ti_custom_menu.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<menu xmlns:android="http://schemas.android.com/apk/res/android">
+    <item


Do we want all this (the android icon, etc)? We just want to disable the normal context items, so don't need placeholder contents here, but likely just an empty menu or something.

I'll remove the files then

sgtcoolguy · 2016-10-11T14:00:33Z

android/titanium/src/java/org/appcelerator/titanium/TiC.java

+	/**
+	 * @module.api
+	 */
+	public static final String PROPERTY_DISABLE_TEXT_SELECTION = "disableTextSelection";


Limit to disableContextMenu to match Hans' iOS implementation.

sgtcoolguy · 2016-10-11T14:00:44Z

android/titanium/src/java/org/appcelerator/titanium/TiC.java

@@ -1598,6 +1613,11 @@
 	/**
 	 * @module.api
 	 */
+	public static final String PROPERTY_IDENTIFIER = "identifier";


What is this for?

Using @hansemannn sample code, I think it was to identify the menu item using user's given id for said item in app.js. Normally an Android developer would have to change the menu xml file but since the user will be using JS, I was going to do it for them programmatically.

ah, ok I found where this came from in the sample. In any case, it was related to the contextMenuItems stuff that we're pushing off to later.

sgtcoolguy · 2016-10-11T14:07:39Z

Does this replace PR #8468?

frankieandone · 2016-10-11T16:44:05Z

Yes I believe so @sgtcoolguy

build · 2016-10-13T23:50:20Z

android/titanium/src/java/org/appcelerator/titanium/TiC.java

@@ -1259,11 +1254,6 @@
 	/**
 	 * @module.api
 	 */
-	public static final String PROPERTY_DISABLE_CONTEXT_MENU = "disableContextMenu";


I think you swapped what we wanted to keep versus cut in your PR. This PR is about implementing disableContextMenu, not disableTextSelection.

fmerzadyan added 3 commits October 7, 2016 11:02

formatted

205772a

specifying disableTextSelection, disableContextMenu and contextMenuIt…

0672d59

…ems properties in app.js

Merge branch 'master' of github.com:appcelerator/titanium_mobile into…

4624a03

… timob-23993

frankieandone changed the title ~~[timob-23993]~~ [TIMOB-23993] Android: Expose disableTextSelection, disableContextMenu, etc in WebView Oct 10, 2016

sgtcoolguy added this to the 6.1.0 milestone Oct 11, 2016

sgtcoolguy added android improvement feature and removed improvement labels Oct 11, 2016

sgtcoolguy suggested changes Oct 11, 2016

View reviewed changes

hansemannn mentioned this pull request Oct 11, 2016

[TIMOB-16369] Android: Disable popup / contextual bar inside webview when text-selecting #8468

Closed

build suggested changes Oct 13, 2016

View reviewed changes

frankieandone force-pushed the timob-23993 branch from 644f534 to 02905b9 Compare October 19, 2016 21:56

cleaned up

01a9497

frankieandone force-pushed the timob-23993 branch from fc5363e to 01a9497 Compare October 19, 2016 22:10

frankieandone changed the title ~~[TIMOB-23993] Android: Expose disableTextSelection, disableContextMenu, etc in WebView~~ [TIMOB-23993] Android: Expose disableContextMenu in TiWebView Oct 27, 2016

sgtcoolguy merged commit 4ed65df into tidev:master Oct 28, 2016

hansemannn mentioned this pull request Nov 7, 2016

[TIMOB-23992] iOS: Disable context menu and contextual menu in Ti.UI.WebView #8488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-23993] Android: Expose disableContextMenu in TiWebView #8494

[TIMOB-23993] Android: Expose disableContextMenu in TiWebView #8494

frankieandone commented Oct 10, 2016 •

edited

sgtcoolguy commented Oct 11, 2016

sgtcoolguy Oct 11, 2016 •

edited

frankieandone Oct 11, 2016

sgtcoolguy Oct 11, 2016 •

edited

frankieandone Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

frankieandone Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy Oct 11, 2016

frankieandone Oct 11, 2016

sgtcoolguy Oct 11, 2016

sgtcoolguy commented Oct 11, 2016

frankieandone commented Oct 11, 2016

build Oct 13, 2016

[TIMOB-23993] Android: Expose disableContextMenu in TiWebView #8494

[TIMOB-23993] Android: Expose disableContextMenu in TiWebView #8494

Conversation

frankieandone commented Oct 10, 2016 • edited

sgtcoolguy commented Oct 11, 2016

sgtcoolguy Oct 11, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgtcoolguy Oct 11, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgtcoolguy commented Oct 11, 2016

frankieandone commented Oct 11, 2016

Choose a reason for hiding this comment

frankieandone commented Oct 10, 2016 •

edited

sgtcoolguy Oct 11, 2016 •

edited

sgtcoolguy Oct 11, 2016 •

edited