Skip to content

Commit

Permalink
Merge pull request #2279 from wordpress-mobile/issue_9832/span_index_…
Browse files Browse the repository at this point in the history
…exceptions

Fix Android Invalid Span Crashes
  • Loading branch information
mchowning authored May 25, 2020
2 parents fe0b27e + 327a2bb commit 7028268
Show file tree
Hide file tree
Showing 28 changed files with 281 additions and 60 deletions.
16 changes: 16 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
1.28.1
------
* [**] Avoid crash when editor selection state becomes invalid

1.28.0
------
* [***] New block: Pullquote
Expand Down
42 changes: 39 additions & 3 deletions __device-tests__/gutenberg-editor-lists.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
Expand All @@ -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 () => {
Expand Down
52 changes: 51 additions & 1 deletion __device-tests__/gutenberg-editor-paragraph.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import EditorPage from './pages/editor-page';
import {
backspace,
setupDriver,
isLocalEnvironment,
clickMiddleOfElement,
Expand Down Expand Up @@ -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 );
Expand All @@ -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 );
Expand Down
5 changes: 4 additions & 1 deletion __device-tests__/helpers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
Expand Down Expand Up @@ -373,6 +375,7 @@ const toggleOrientation = async ( driver: wd.PromiseChainWebdriver ) => {
};

module.exports = {
backspace,
timer,
setupDriver,
isLocalEnvironment,
Expand Down
5 changes: 3 additions & 2 deletions android/app/src/main/java/com/gutenberg/MainApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down Expand Up @@ -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);
}
Expand Down
22 changes: 11 additions & 11 deletions bundle/android/App.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/App.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/raw/i18ncache_data_de.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/raw/i18ncache_data_hr.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/raw/i18ncache_data_it.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/raw/i18ncache_data_pl.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bundle/android/raw/i18ncache_data_ru.json

Large diffs are not rendered by default.

Loading

0 comments on commit 7028268

Please sign in to comment.