Fix crash on mobile when pasting formatted text. #18740
Conversation
… mobile when pasting formatted text.
cameronvoell
requested review from
ellatrix,
gziolo and
youknowriad
as code owners
cameronvoell
requested review from
etoledom
and removed request for
youknowriad and
gziolo
cameronvoell
added
the
Mobile App - i.e. Android or iOS
There was a problem hiding this comment.
Looks good and safe to me! Thank you @cameronvoell for figuring this out.
Just to be sure, let's wait for @ellatrix 's 👀 before merging.
|
I don't see anything at https://app.circleci.com/jobs/github/wordpress-mobile/gutenberg-mobile/17951/tests. |
| @@ -42,7 +42,7 @@ export default function( node ) { | |||
| } | |||
|
|
|||
| // Ignore pre content. | |||
| if ( node.parentElement.closest( 'pre' ) ) { | |||
| if ( node.parentElement.closest && node.parentElement.closest( 'pre' ) ) { | |||
There was a problem hiding this comment.
What is node.parentElement if not an element node or null? Could you add a test case or leave a comment?
There was a problem hiding this comment.
@ellatrix - Do you mean a comment explaining the reason?
For what I understand, this check prevents a crash on Native Android. Would that be good for a comment?
There was a problem hiding this comment.
I would like to know the cause of the crash. What is node.parentElement, if not an Element? This line important for paste. We don't want it to be skipped, even for mobile.
https://developer.mozilla.org/en-US/docs/Web/API/Element/closest
There was a problem hiding this comment.
There is this new PR that we hope will solve the root cause:
wordpress-mobile/gutenberg-mobile#1626
If that's the case, this PR won't be needed anymore 🙏
You're right @ellatrix . Unfortunately in this case, the failing tests are in the e2e native tests and those are not in the Gutenberg repo yet. Just for adding context, the e2e tests involve testing on real devices/emulators and we are not ready yet to include those in Gutenberg's repo. In the meantime, wordpress-mobile/gutenberg-mobile#1626 got merged and fixes the issue via a different route so, I think it's OK to close this PR now @cameronvoell . |
Description
Adding a check for undefined
node.parentElement.closestwhich was caught in this failing test causing a crash on mobile when pasting formatted (bold) textHow has this been tested?
I tested on gutenberg web, and was not able to reproduce the error that I was seeing on mobile. I tested that the check for
undefinedfixes the pasting of formatted text crash on mobile.See Gutenberg mobile PR is now showing a pass on the test for pasting formatted text: wordpress-mobile/gutenberg-mobile#1617
Screenshots
Types of changes
Checklist: