From ff260c03caedeeaaf00563e117faefb8004f269b Mon Sep 17 00:00:00 2001 From: Ian Clelland Date: Tue, 24 Jun 2014 13:49:12 -0400 Subject: [PATCH 1/5] CB-5971: Factor out package/project name validation logic --- bin/lib/create.js | 78 +++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/bin/lib/create.js b/bin/lib/create.js index f99d1da9..8a908276 100755 --- a/bin/lib/create.js +++ b/bin/lib/create.js @@ -106,6 +106,50 @@ function copyScripts(projectPath) { shell.cp(path.join(ROOT, 'bin', 'lib', 'android_sdk_version.js'), path.join(projectPath, 'cordova', 'lib', 'android_sdk_version.js')); } +/** + * Test whether a package name is acceptable for use as an android project. + * Returns a promise, fulfilled if the package name is acceptable; rejected + * otherwise. + */ +function validatePackageName(package_name) { + //Make the package conform to Java package types + if (!/[a-zA-Z0-9_]+\.[a-zA-Z0-9_](.[a-zA-Z0-9_])*/.test(package_name)) { + return Q.reject('Package name must look like: com.company.Name'); + } + + //Enforce underscore limitation + if (/[_]+[a-zA-Z0-9_]*/.test(package_name)) { + return Q.reject("Package name can't begin with an underscore"); + } + + //Class is a reserved word + if(/[C|c]+lass+[\s|\.]/.test(package_name) && !/[a-zA-Z0-9_]+[C|c]+lass/.test(package_name)) + { + return Q.reject('class is a reserved word'); + } + + return Q.resolve(); +} + +/** + * Test whether a project name is acceptable for use as an android class. + * Returns a promise, fulfilled if the project name is acceptable; rejected + * otherwise. + */ +function validateProjectName(project_name) { + //Enforce stupid name error + if (project_name === 'CordovaActivity') { + return Q.reject('Project name cannot be CordovaActivity'); + } + + //Classes in Java don't begin with numbers + if (/^[0-9]/.test(project_name)) { + return Q.reject('Project name must not begin with a number'); + } + + return Q.resolve(); +} + /** * $ create [options] * @@ -146,34 +190,14 @@ exports.createProject = function(project_path, package_name, project_name, proje } //Make the package conform to Java package types - if (!/[a-zA-Z0-9_]+\.[a-zA-Z0-9_](.[a-zA-Z0-9_])*/.test(package_name)) { - return Q.reject('Package name must look like: com.company.Name'); - } - - //Enforce underscore limitation - if (/[_]+[a-zA-Z0-9_]*/.test(package_name)) { - return Q.reject("Package name can't begin with an underscore"); - } - - //Enforce stupid name error - if (project_name === 'CordovaActivity') { - return Q.reject('Project name cannot be CordovaActivity'); - } - - //Classes in Java don't begin with numbers - if (/[0-9]+[a-zA-Z0-9]/.test(project_name)) { - return Q.reject('Project name must not begin with a number'); - } - - //Class is a reserved word - if(/[C|c]+lass+[\s|\.]/.test(package_name) && !/[a-zA-Z0-9_]+[C|c]+lass/.test(package_name)) - { - return Q.reject('class is a reserved word'); - } - - // Check that requirements are met and proper targets are installed - return check_reqs.run() + return validatePackageName(package_name) .then(function() { + validateProjectName(project_name); + }) + // Check that requirements are met and proper targets are installed + .then(function() { + check_reqs.run(); + }).then(function() { // Log the given values for the project console.log('Creating Cordova project for the Android platform:'); console.log('\tPath: ' + project_path); From bb141a70e89cf7826fb1699085df33aac597a688 Mon Sep 17 00:00:00 2001 From: Ian Clelland Date: Tue, 24 Jun 2014 13:50:00 -0400 Subject: [PATCH 2/5] CB-5971: Add unit tests to cordova-android --- bin/lib/create.js | 4 +++ package.json | 7 ++++ spec/create.spec.js | 82 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 spec/create.spec.js diff --git a/bin/lib/create.js b/bin/lib/create.js index 8a908276..a31604a4 100755 --- a/bin/lib/create.js +++ b/bin/lib/create.js @@ -288,3 +288,7 @@ exports.updateProject = function(projectPath) { }); }; + +// For testing +exports.validatePackageName = validatePackageName; +exports.validateProjectName = validateProjectName; diff --git a/package.json b/package.json index 0d203c92..ed01458a 100644 --- a/package.json +++ b/package.json @@ -12,10 +12,17 @@ "cordova", "apache" ], + "scripts": { + "test": "jasmine-node --color spec" + }, "author": "Apache Software Foundation", "license": "Apache version 2.0", "dependencies": { "q": "^0.9.0", "shelljs": "^0.2.6" + }, + "devDependencies": { + "jasmine-node": "~1", + "promise-matchers": "~0" } } diff --git a/spec/create.spec.js b/spec/create.spec.js new file mode 100644 index 00000000..8e08793f --- /dev/null +++ b/spec/create.spec.js @@ -0,0 +1,82 @@ +/** + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +*/ +/* jshint laxcomma:true */ + +require("promise-matchers"); + +var create = require("../bin/lib/create"); + +describe("create", function () { + describe("validatePackageName", function() { + var valid = [ + "org.apache.mobilespec" + , "com.example" + , "com.42floors.package" + ]; + var invalid = [ + "" + , "com.class.is.bad" + , "0com.example.mobilespec" + , "c-m.e@a!p%e.mobilespec" + , "notenoughdots" + , ".starts.with.a.dot" + , "ends.with.a.dot." + , "_underscore.anything" + , "underscore._something" + , "_underscore._all._the._things" + ]; + + valid.forEach(function(package_name) { + it("should accept " + package_name, function(done) { + expect(create.validatePackageName(package_name)).toHaveBeenResolved(done); + }); + }); + + invalid.forEach(function(package_name) { + it("should reject " + package_name, function(done) { + expect(create.validatePackageName(package_name)).toHaveBeenRejected(done); + }); + }); + }); + describe("validateProjectName", function() { + var valid = [ + "mobilespec" + , "package_name" + , "PackageName" + , "CordovaLib" + ]; + var invalid = [ + "" + , "0startswithdigit" + , "CordovaActivity" + ]; + + valid.forEach(function(project_name) { + it("should accept " + project_name, function(done) { + expect(create.validateProjectName(project_name)).toHaveBeenResolved(done); + }); + }); + + invalid.forEach(function(project_name) { + it("should reject " + project_name, function(done) { + expect(create.validateProjectName(project_name)).toHaveBeenRejected(done); + }); + }); + }); +}); From 4352456129bfa8a8a6bbf4d38fb40d9876b67ec4 Mon Sep 17 00:00:00 2001 From: Ian Clelland Date: Tue, 24 Jun 2014 14:05:03 -0400 Subject: [PATCH 3/5] CB-5971: Fix package / project validation --- bin/lib/create.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bin/lib/create.js b/bin/lib/create.js index a31604a4..b2490f24 100755 --- a/bin/lib/create.js +++ b/bin/lib/create.js @@ -113,18 +113,13 @@ function copyScripts(projectPath) { */ function validatePackageName(package_name) { //Make the package conform to Java package types - if (!/[a-zA-Z0-9_]+\.[a-zA-Z0-9_](.[a-zA-Z0-9_])*/.test(package_name)) { + //Enforce underscore limitation + if (!/^[a-zA-Z]+(\.[a-zA-Z0-9][a-zA-Z0-9_]*)+$/.test(package_name)) { return Q.reject('Package name must look like: com.company.Name'); } - //Enforce underscore limitation - if (/[_]+[a-zA-Z0-9_]*/.test(package_name)) { - return Q.reject("Package name can't begin with an underscore"); - } - //Class is a reserved word - if(/[C|c]+lass+[\s|\.]/.test(package_name) && !/[a-zA-Z0-9_]+[C|c]+lass/.test(package_name)) - { + if(/\b[Cc]lass\b/.test(package_name)) { return Q.reject('class is a reserved word'); } @@ -137,6 +132,11 @@ function validatePackageName(package_name) { * otherwise. */ function validateProjectName(project_name) { + //Make sure there's something there + if (project_name === '') { + return Q.reject('Project name cannot be empty'); + } + //Enforce stupid name error if (project_name === 'CordovaActivity') { return Q.reject('Project name cannot be CordovaActivity'); From 58afd0b6040b086cfe32e96e1438b4c2f9d23743 Mon Sep 17 00:00:00 2001 From: Andrew Grieve Date: Tue, 24 Jun 2014 14:55:34 -0400 Subject: [PATCH 4/5] CB-7017 Fix onload=true being set on all subsequent plugins --- framework/src/org/apache/cordova/PluginManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/framework/src/org/apache/cordova/PluginManager.java b/framework/src/org/apache/cordova/PluginManager.java index f0957222..1ed05237 100755 --- a/framework/src/org/apache/cordova/PluginManager.java +++ b/framework/src/org/apache/cordova/PluginManager.java @@ -165,6 +165,7 @@ public class PluginManager { service = ""; pluginClass = ""; insideFeature = false; + onload = false; } } try { From 4b4a2e9f9e6ebcef57cee2e5f4c44108ec72ca25 Mon Sep 17 00:00:00 2001 From: Andrew Grieve Date: Tue, 24 Jun 2014 15:08:47 -0400 Subject: [PATCH 5/5] CB-7018 Clean up and deprecation of some button-related functions --- framework/src/org/apache/cordova/App.java | 12 ++- .../org/apache/cordova/CordovaWebView.java | 81 +++++++++---------- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/framework/src/org/apache/cordova/App.java b/framework/src/org/apache/cordova/App.java index 4b15906d..21125194 100755 --- a/framework/src/org/apache/cordova/App.java +++ b/framework/src/org/apache/cordova/App.java @@ -32,6 +32,7 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.telephony.TelephonyManager; +import android.view.KeyEvent; import java.util.HashMap; @@ -217,7 +218,7 @@ public class App extends CordovaPlugin { */ public void overrideBackbutton(boolean override) { LOG.i("App", "WARNING: Back Button Default Behavior will be overridden. The backbutton event will be fired!"); - webView.bindButton(override); + webView.setButtonPlumbedToJs(KeyEvent.KEYCODE_BACK, override); } /** @@ -229,7 +230,12 @@ public class App extends CordovaPlugin { */ public void overrideButton(String button, boolean override) { LOG.i("App", "WARNING: Volume Button Default Behavior will be overridden. The volume event will be fired!"); - webView.bindButton(button, override); + if (button.equals("volumeup")) { + webView.setButtonPlumbedToJs(KeyEvent.KEYCODE_VOLUME_UP, override); + } + else if (button.equals("volumedown")) { + webView.setButtonPlumbedToJs(KeyEvent.KEYCODE_VOLUME_DOWN, override); + } } /** @@ -238,7 +244,7 @@ public class App extends CordovaPlugin { * @return boolean */ public boolean isBackbuttonOverridden() { - return webView.isBackButtonBound(); + return webView.isButtonPlumbedToJs(KeyEvent.KEYCODE_BACK); } /** diff --git a/framework/src/org/apache/cordova/CordovaWebView.java b/framework/src/org/apache/cordova/CordovaWebView.java index 3b3f80d3..b31eec57 100755 --- a/framework/src/org/apache/cordova/CordovaWebView.java +++ b/framework/src/org/apache/cordova/CordovaWebView.java @@ -21,8 +21,8 @@ package org.apache.cordova; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.Locale; import org.apache.cordova.Config; @@ -70,8 +70,7 @@ public class CordovaWebView extends WebView { public static final String TAG = "CordovaWebView"; public static final String CORDOVA_VERSION = "3.6.0-dev"; - private ArrayList keyDownCodes = new ArrayList(); - private ArrayList keyUpCodes = new ArrayList(); + private HashSet boundKeyCodes = new HashSet(); public PluginManager pluginManager; private boolean paused; @@ -90,10 +89,6 @@ public class CordovaWebView extends WebView { // Flag to track that a loadUrl timeout occurred int loadUrlTimeout = 0; - private boolean bound; - - private boolean handleButton = false; - private long lastMenuEventTime = 0; NativeToJsMessageQueue jsMessageQueue; @@ -705,23 +700,17 @@ public class CordovaWebView extends WebView { return p.toString(); } - /* - * onKeyDown - */ @Override public boolean onKeyDown(int keyCode, KeyEvent event) { - if(keyDownCodes.contains(keyCode)) + if(boundKeyCodes.contains(keyCode)) { if (keyCode == KeyEvent.KEYCODE_VOLUME_DOWN) { - // only override default behavior is event bound - LOG.d(TAG, "Down Key Hit"); this.loadUrl("javascript:cordova.fireDocumentEvent('volumedownbutton');"); return true; } // If volumeup key else if (keyCode == KeyEvent.KEYCODE_VOLUME_UP) { - LOG.d(TAG, "Up Key Hit"); this.loadUrl("javascript:cordova.fireDocumentEvent('volumeupbutton');"); return true; } @@ -732,7 +721,7 @@ public class CordovaWebView extends WebView { } else if(keyCode == KeyEvent.KEYCODE_BACK) { - return !(this.startOfHistory()) || this.bound; + return !(this.startOfHistory()) || isButtonPlumbedToJs(KeyEvent.KEYCODE_BACK); } else if(keyCode == KeyEvent.KEYCODE_MENU) { @@ -749,10 +738,8 @@ public class CordovaWebView extends WebView { return super.onKeyDown(keyCode, event); } } - return super.onKeyDown(keyCode, event); } - @Override public boolean onKeyUp(int keyCode, KeyEvent event) @@ -762,10 +749,11 @@ public class CordovaWebView extends WebView { // A custom view is currently displayed (e.g. playing a video) if(mCustomView != null) { this.hideCustomView(); + return true; } else { // The webview is currently displayed // If back key is bound, then send event to JavaScript - if (this.bound) { + if (isButtonPlumbedToJs(KeyEvent.KEYCODE_BACK)) { this.loadUrl("javascript:cordova.fireDocumentEvent('backbutton');"); return true; } else { @@ -791,48 +779,56 @@ public class CordovaWebView extends WebView { this.loadUrl("javascript:cordova.fireDocumentEvent('searchbutton');"); return true; } - else if(keyUpCodes.contains(keyCode)) - { - //What the hell should this do? - return super.onKeyUp(keyCode, event); - } //Does webkit change this behavior? return super.onKeyUp(keyCode, event); } - + public void setButtonPlumbedToJs(int keyCode, boolean value) { + switch (keyCode) { + case KeyEvent.KEYCODE_VOLUME_DOWN: + case KeyEvent.KEYCODE_VOLUME_UP: + case KeyEvent.KEYCODE_BACK: + // TODO: Why are search and menu buttons handled separately? + boundKeyCodes.add(keyCode); + return; + default: + throw new IllegalArgumentException("Unsupported keycode: " + keyCode); + } + } + + @Deprecated // Use setButtonPlumbedToJs() instead. public void bindButton(boolean override) { - this.bound = override; + setButtonPlumbedToJs(KeyEvent.KEYCODE_BACK, override); } + @Deprecated // Use setButtonPlumbedToJs() instead. public void bindButton(String button, boolean override) { - // TODO Auto-generated method stub if (button.compareTo("volumeup")==0) { - keyDownCodes.add(KeyEvent.KEYCODE_VOLUME_UP); + setButtonPlumbedToJs(KeyEvent.KEYCODE_VOLUME_UP, override); } else if (button.compareTo("volumedown")==0) { - keyDownCodes.add(KeyEvent.KEYCODE_VOLUME_DOWN); + setButtonPlumbedToJs(KeyEvent.KEYCODE_VOLUME_DOWN, override); } - } - - public void bindButton(int keyCode, boolean keyDown, boolean override) { - if(keyDown) - { - keyDownCodes.add(keyCode); - } - else - { - keyUpCodes.add(keyCode); - } } + @Deprecated // Use setButtonPlumbedToJs() instead. + public void bindButton(int keyCode, boolean keyDown, boolean override) { + setButtonPlumbedToJs(keyCode, override); + } + + @Deprecated // Use isButtonPlumbedToJs public boolean isBackButtonBound() { - return this.bound; + return isButtonPlumbedToJs(KeyEvent.KEYCODE_BACK); } - + + public boolean isButtonPlumbedToJs(int keyCode) + { + return boundKeyCodes.contains(keyCode); + } + public void handlePause(boolean keepRunning) { LOG.d(TAG, "Handle the pause"); @@ -904,8 +900,9 @@ public class CordovaWebView extends WebView { return paused; } + @Deprecated // This never did anything. public boolean hadKeyEvent() { - return handleButton; + return false; } // Wrapping these functions in their own class prevents warnings in adb like: