Merge pull request #2310 from wordpress-mobile/release/1.28.1

Hotfix Release 1.28.1

.circleci/config.yml

@@ -105,6 +105,20 @@ jobs:
                 include_job_number_field: false
                 include_project_field: false
                 failure_message: ':red_circle: Scheduled tests failed on Android device!'
+  android-native-unit-tests:
+    parameters:
+      post-to-slack:
+        description: Post to Slack when tests fail. SLACK_WEBHOOK ENV variable must be set.
+        type: boolean
+        default: false
+    docker:
+    - image: circleci/android:api-29-node
+    steps:
+      - checkout
+      - yarn-install
+      - run:
+          name: Run Android native unit tests
+          command: cd android && ./gradlew testDebug
   ios-device-checks:
     parameters:
       post-to-slack:
@@ -204,6 +218,8 @@ workflows:
           name: Test iOS on Device
       - android-device-checks:
           name: Test Android on Device
+      - android-native-unit-tests:
+          name: Android Native Unit Tests
 
   ui-tests-full-scheduled:
     jobs:

RELEASE-NOTES.txt

@@ -1,3 +1,7 @@
+1.28.1
+------
+* [**] Avoid crash when editor selection state becomes invalid
+
 1.28.0
 ------
 * [***] New block: Pullquote

__device-tests__/gutenberg-editor-lists.test.js

@@ -7,10 +7,11 @@
  */
 import EditorPage from './pages/editor-page';
 import {
-	setupDriver,
+	backspace,
+	isAndroid,
 	isLocalEnvironment,
+	setupDriver,
 	stopDriver,
-	isAndroid,
 } from './helpers/utils';
 import testData from './helpers/test-data';
 
@@ -63,8 +64,9 @@ describe( 'Gutenberg Editor tests for List block', () => {
 		await editorPage.verifyHtmlContent( testData.listHtml );
 	} );
 
+	// This test depends on being run immediately after 'should be able to add a new List block'
 	it( 'should update format to ordered list, using toolbar button', async () => {
-		const listBlockElement = await editorPage.getBlockAtPosition( listBlockName );
+		let listBlockElement = await editorPage.getBlockAtPosition( listBlockName );
 
 		// Click List block to force EditText focus
 		await listBlockElement.click();
@@ -74,6 +76,40 @@ describe( 'Gutenberg Editor tests for List block', () => {
 
 		// switch to html and verify html
 		await editorPage.verifyHtmlContent( testData.listHtmlOrdered );
+
+		// Remove list block to return editor to empty state
+		listBlockElement = await editorPage.getBlockAtPosition( listBlockName );
+		await listBlockElement.click();
+		await editorPage.removeBlockAtPosition( listBlockName );
+	} );
+
+	// Prevent regression of https://github.com/wordpress-mobile/gutenberg-mobile/issues/871
+	it( 'should handle spaces in a list', async () => {
+		await editorPage.addNewBlock( listBlockName );
+		let listBlockElement = await editorPage.getBlockAtPosition( listBlockName );
+		// Click List block on Android to force EditText focus
+		if ( isAndroid() ) {
+			await listBlockElement.click();
+		}
+
+		// Send the list item text
+		await editorPage.sendTextToListBlock( listBlockElement, '  a' );
+
+		// send an Enter
+		await editorPage.sendTextToListBlock( listBlockElement, '\n' );
+
+		// send a backspace
+		await editorPage.sendTextToListBlock( listBlockElement, backspace );
+
+		// switch to html and verify html
+		await editorPage.verifyHtmlContent( `<!-- wp:list -->
+<ul><li>  a</li></ul>
+<!-- /wp:list -->` );
+
+		// Remove list block to reset editor to clean state
+		listBlockElement = await editorPage.getBlockAtPosition( listBlockName );
+		await listBlockElement.click();
+		await editorPage.removeBlockAtPosition( listBlockName );
 	} );
 
 	afterAll( async () => {

__device-tests__/gutenberg-editor-paragraph.test.js

@@ -7,6 +7,7 @@
  */
 import EditorPage from './pages/editor-page';
 import {
+	backspace,
 	setupDriver,
 	isLocalEnvironment,
 	clickMiddleOfElement,
@@ -103,7 +104,7 @@ describe( 'Gutenberg Editor tests for Paragraph Block', () => {
 
 		textViewElement = await editorPage.getTextViewForParagraphBlock( paragraphBlockElement );
 		await clickBeginningOfElement( driver, textViewElement );
-		await editorPage.typeTextToParagraphBlock( paragraphBlockElement, '\u0008' );
+		await editorPage.typeTextToParagraphBlock( paragraphBlockElement, backspace );
 
 		const text = await editorPage.getTextForParagraphBlockAtPosition( 1 );
 		expect( text0 + text1 ).toMatch( text );
@@ -127,6 +128,55 @@ describe( 'Gutenberg Editor tests for Paragraph Block', () => {
 		}
 	} );
 
+	// Restricting these test to Android because I was not able to update the html on iOS
+	if ( isAndroid() ) {
+		it( 'should be able to merge blocks with unknown html elements', async () => {
+			await editorPage.setHtmlContentAndroid( `
+<!-- wp:paragraph -->
+<p><unknownhtmlelement>abc</unknownhtmlelement>D</p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p>E</p>
+<!-- /wp:paragraph -->` );
+
+			// // Merge paragraphs
+			const secondParagraphBlockElement = await editorPage.getBlockAtPosition( paragraphBlockName, 2 );
+			await clickBeginningOfElement( driver, secondParagraphBlockElement );
+			await editorPage.typeTextToParagraphBlock( secondParagraphBlockElement, backspace );
+
+			// verify the editor has not crashed
+			const text = await editorPage.getTextForParagraphBlockAtPosition( 1 );
+			expect( text.length ).not.toEqual( 0 );
+
+			await editorPage.removeBlockAtPosition( paragraphBlockName );
+		} );
+
+		// Based on https://github.com/wordpress-mobile/gutenberg-mobile/pull/1507
+		it( 'should handle multiline paragraphs from web', async () => {
+			await editorPage.setHtmlContentAndroid( `
+	<!-- wp:paragraph -->
+	<p>multiple lines<br><br></p>
+	<!-- /wp:paragraph -->
+
+	<!-- wp:paragraph -->
+	<p></p>
+	<!-- /wp:paragraph -->`
+			);
+
+			// // Merge paragraphs
+			const secondParagraphBlockElement = await editorPage.getBlockAtPosition( paragraphBlockName, 2 );
+			await clickBeginningOfElement( driver, secondParagraphBlockElement );
+			await editorPage.typeTextToParagraphBlock( secondParagraphBlockElement, backspace );
+
+			// verify the editor has not crashed
+			const text = await editorPage.getTextForParagraphBlockAtPosition( 1 );
+			expect( text.length ).not.toEqual( 0 );
+
+			await editorPage.removeBlockAtPosition( paragraphBlockName );
+		} );
+	}
+
 	afterAll( async () => {
 		if ( ! isLocalEnvironment() ) {
 			driver.sauceJobStatus( allPassed );

__device-tests__/helpers/utils.js

@@ -37,11 +37,13 @@ const localIOSAppPath = process.env.IOS_APP_PATH || defaultIOSAppPath;
 const localAppiumPort = serverConfigs.local.port; // Port to spawn appium process for local runs
 let appiumProcess: ?childProcess.ChildProcess;
 
+const backspace = '\u0008';
+
 // Used to map unicode and special values to keycodes on Android
 // Docs for keycode values: https://developer.android.com/reference/android/view/KeyEvent.html
 const strToKeycode = {
 	'\n': 66,
-	'\u0008': 67,
+	[ backspace ]: 67,
 };
 
 const timer = ( ms: number ) => new Promise < {} > ( ( res ) => setTimeout( res, ms ) );
@@ -373,6 +375,7 @@ const toggleOrientation = async ( driver: wd.PromiseChainWebdriver ) => {
 };
 
 module.exports = {
+	backspace,
 	timer,
 	setupDriver,
 	isLocalEnvironment,

android/app/src/main/java/com/gutenberg/MainApplication.java

@@ -63,7 +63,7 @@ public void requestMediaPickFromMediaLibrary(MediaUploadCallback mediaUploadCall
                 } else if (mediaType == MediaType.VIDEO) {
                     rnMediaList.add(new Media(2, "https://i.cloudup.com/YtZFJbuQCE.mov", "video", "Cloudup" ));
                 }
-                mediaUploadCallback.onUploadMediaFileSelected(rnMediaList);                
+                mediaUploadCallback.onUploadMediaFileSelected(rnMediaList);
             }
 
 
@@ -151,7 +151,8 @@ protected List<ReactPackage> getPackages() {
                         new ReactSliderPackage(),
                         new ReactVideoPackage(),
                         new SvgPackage(),
-                        new ReactAztecPackage(),
+                        // passing null because we do not need log handlers in the demo app
+                        new ReactAztecPackage(null, null),
                         new LinearGradientPackage(),
                         mRnReactNativeGutenbergBridgePackage);
             }

ios/Podfile.lock

@@ -21,7 +21,7 @@ PODS:
     - DoubleConversion
     - glog
   - glog (0.3.5)
-  - Gutenberg (1.28.0):
+  - Gutenberg (1.28.1):
     - React (= 0.61.5)
     - React-CoreModules (= 0.61.5)
     - React-RCTImage (= 0.61.5)
@@ -239,7 +239,7 @@ PODS:
     - React
   - RNSVG (9.13.6-gb):
     - React
-  - RNTAztecView (1.28.0):
+  - RNTAztecView (1.28.1):
     - React-Core
     - WordPress-Aztec-iOS (~> 1.19.0)
   - WordPress-Aztec-iOS (1.19.0)
@@ -367,7 +367,7 @@ SPEC CHECKSUMS:
   FBReactNativeSpec: 118d0d177724c2d67f08a59136eb29ef5943ec75
   Folly: 30e7936e1c45c08d884aa59369ed951a8e68cf51
   glog: 1f3da668190260b06b429bb211bfbee5cd790c28
-  Gutenberg: 0c90bd47ecf991fbe677172a3a2f8ab1ef715c07
+  Gutenberg: 76149a3f98afe2ef0b0293789b26881a676b07a2
   RCTRequired: b153add4da6e7dbc44aebf93f3cf4fcae392ddf1
   RCTTypeSafety: 9aa1b91d7f9310fc6eadc3cf95126ffe818af320
   React: b6a59ef847b2b40bb6e0180a97d0ca716969ac78
@@ -393,7 +393,7 @@ SPEC CHECKSUMS:
   ReactCommon: 198c7c8d3591f975e5431bec1b0b3b581aa1c5dd
   ReactNativeDarkMode: f61376360c5d983907e5c316e8e1c853a8c2f348
   RNSVG: 68a534a5db06dcbdaebfd5079349191598caef7b
-  RNTAztecView: c8face301ee453aab24e9e10615353134cca505b
+  RNTAztecView: 8ed1c3f920a3f8cb1852e15bba6847db75f9bc79
   WordPress-Aztec-iOS: fb6ea6409a5228292568f665eb22ea0a0aa7ad7e
   Yoga: f2a7cd4280bfe2cca5a7aed98ba0eb3d1310f18b
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gutenberg-mobile",
-  "version": "1.28.0",
+  "version": "1.28.1",
   "private": true,
   "config": {
     "jsfiles": "./*.js src/*.js src/**/*.js src/**/**/*.js",

react-native-aztec/android/build.gradle

@@ -109,6 +109,7 @@ dependencies {
     implementation 'androidx.cardview:cardview:1.0.0'
     implementation 'androidx.appcompat:appcompat:1.0.0'
     implementation 'androidx.recyclerview:recyclerview:1.0.0'
+    testImplementation 'junit:junit:4.12'
 
     if (rootProject.ext.buildGutenbergFromSource) {
         implementation "com.facebook.react:react-native:+" // From node_modules.

react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/IllegalSelectionIndexException.java

@@ -0,0 +1,49 @@
+package org.wordpress.mobile.ReactNativeAztec;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
+
+import org.jetbrains.annotations.NotNull;
+import org.wordpress.aztec.spans.UnknownHtmlSpan;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+class IllegalSelectionIndexException extends Exception {
+    IllegalSelectionIndexException(int selectionStart, int selectionEnd, int textLength, ReactAztecText view) {
+        super(createMessage(selectionStart, selectionEnd, textLength, getUnknownHtmlTags(view, textLength)));
+    }
+
+    private static String createMessage(int selectionStart,
+                                        int selectionEnd,
+                                        int textLength,
+                                        List<String> unknownHtmlTags) {
+        return "Illegal selection index for text with length " + textLength +
+               ", selectionStart: " + selectionStart +
+               ", selectionEnd: " + selectionEnd +
+               ", with " + UnknownHtmlSpan.class.getSimpleName() + " tags: " + unknownHtmlTags;
+    }
+
+    @NotNull
+    private static List<String> getUnknownHtmlTags(ReactAztecText view, int textLength) {
+        List<String> unknownHtmlTags = new ArrayList<>();
+        for (UnknownHtmlSpan span : view.getText().getSpans(0, textLength, UnknownHtmlSpan.class)) {
+            String rawHtml = span.getRawHtml().toString();
+            unknownHtmlTags.addAll(parseTags(rawHtml));
+        }
+        return unknownHtmlTags;
+    }
+
+    @VisibleForTesting
+    @NonNull
+    static List<String> parseTags(String html) {
+        List<String> tags = new ArrayList<>();
+        Matcher matcher = Pattern.compile("<([^\\\\s>/]+)>").matcher(html);
+        while (matcher.find()) {
+            tags.add(matcher.group(1));
+        }
+        return tags;
+    }
+}

react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java

@@ -6,6 +6,7 @@
 import android.os.Build;
 import androidx.annotation.Nullable;
 import androidx.core.graphics.ColorUtils;
+import androidx.core.util.Consumer;
 
 import android.os.Handler;
 import android.os.Looper;
@@ -76,7 +77,12 @@ public class ReactAztecManager extends BaseViewManager<ReactAztecText, LayoutSha
 
     private static final String BLOCK_TYPE_TAG_KEY = "tag";
 
-    public ReactAztecManager() {
+    @Nullable private final Consumer<Exception> exceptionLogger;
+    @Nullable private final Consumer<String> breadcrumbLogger;
+
+    public ReactAztecManager(@Nullable Consumer<Exception> exceptionLogger, @Nullable Consumer<String> breadcrumbLogger) {
+        this.exceptionLogger = exceptionLogger;
+        this.breadcrumbLogger = breadcrumbLogger;
         initializeFocusAndBlurCommandCodes();
     }
 
@@ -222,7 +228,31 @@ private void updateSelectionIfNeeded(ReactAztecText view, @Nullable ReadableMap
         if ( selection != null ) {
             int start = selection.getInt("start");
             int end = selection.getInt("end");
-            view.setSelection(start, end);
+            int textLength = view.getText().length();
+            boolean startAndEndAreValid = start >= 0 &&
+                                          end >= 0 &&
+                                          start <= textLength &&
+                                          end <= textLength;
+            if (startAndEndAreValid) {
+                view.setSelection(start, end);
+            } else {
+                // Calling view.setSelection would have thrown an exception, so let's send information about
+                // what happened to help us figure out how we got into a bad state.
+                try {
+                    IllegalSelectionIndexException customException =
+                            new IllegalSelectionIndexException(start, end, textLength, view);
+                    customException.printStackTrace();
+                    if (exceptionLogger != null) {
+                        exceptionLogger.accept(customException);
+                    }
+                    if (breadcrumbLogger != null) {
+                        breadcrumbLogger.accept(customException.getMessage());
+                    }
+                } catch (Exception e) {
+                    // Should never happen, but if it does don't let logging cause a crash
+                    e.printStackTrace();
+                }
+            }
         }
     }