RNMobile: Remove willTrimSpacesCheck for Android #22006
Conversation
mchowning
added
the
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 825 kB ℹ️ View Unchanged
|
mchowning
force-pushed
the
rnmobile/remove_will_trim_check_for_android
branch
from
9ed4b48 to
3208103
Compare
| const blockHtmlElements = | ||
| '(div|br|blockquote|ul|ol|li|p|pre|h1|h2|h3|h4|h5|h6|iframe|hr)'; | ||
| const leadingOrTrailingSpaces = new RegExp( | ||
| `(\\s+)<\/?${ blockHtmlElements }>|<\/?${ blockHtmlElements }>(\\s+)`, |
There was a problem hiding this comment.
👋 @mchowning, looks like the whitespace preserving code on the Aztec side is preserving simple spaces only, while the regex here is checking for all whitespace chars. I'm under the impression that the Aztec side behavior won't actually preserve all whitespace chars, so, can you check as well?
I tried having a tab char (ASCII 0x09) leading and trialing the test but couldn't break the editor (the whitespace was preserved) although I'm not sure I did actually manage to insert the TAB, it might actually had become a regular space char.
There was a problem hiding this comment.
Thanks for taking a look @hypest!
looks like the whitespace preserving code on the Aztec side is preserving simple spaces only, while the regex here is checking for all whitespace chars.
Are you aware of any specific reason the regex is checking for all whitespace instead of just spaces. I wasn't able to find a reason that was needed (both the comment on the regex and the original change with adding this regex seemed to only involve issues with space characters specifically.
I'm under the impression that the Aztec side behavior won't actually preserve all whitespace chars, so, can you check as well?
I tried having a tab char (ASCII 0x09) leading and trialing the test but couldn't break the editor (the whitespace was preserved) although I'm not sure I did actually manage to insert the TAB, it might actually had become a regular space char.
I noticed the same thing, a lot of non-standard characters I tried (tab, line feed, carriage return, non-breaking space) were converted into spaces (well, eventually: line feed would briefly be a newline and non-breaking space would briefly be , but those would then get turned into spaces). In everything I tried, I couldn't find anything that broke on Gutenberg.
There was a problem hiding this comment.
Are you aware of any specific reason the regex is checking for all whitespace instead of just spaces.
If I recall correctly, the whitespace in general gets removed by Aztec because HTML itself is ignoring them, and Aztec tries to follow that logic. In this case, I think I tried to generalize and having in mind that the AztecParser.tidy() method and perhaps other components too will try to remove some whitespace.
|
Closing this because these changes were handled in wordpress-mobile/gutenberg-mobile#2279 |
Description
This removes the
willTrimSpacescheck that Android has been using inRichTextso that we are not discarding the selection information as often on Android. I believe we can now remove this on account of @marecar3 's update so that AztecAndroid is no longer discarding whitespace like it was before. (AztecAndroid PR, Gutenberg-Mobile PR).The immediate issue that brought this up is @iamthomasbishop 's request that when inserting @-mentions into a post, we include a space after the mention. Doing this on Android triggers the
willTrimSpacescheck and moves the cursor from the end of the mention to the beginning of the mention, which is a significant disruption to the writing flow (more info in this comment).This check was added to avoid a number of crashes on Android, so we should be very cautious about removing it and thoroughly test for regressions. 🙏 I searched for crashes we saw in the past and included them in the testing scenarios below.
Testing Scenarios
Confirm that all of these flows are able to be completed crash-free
Multiline Paragraph Blocks From Web
From wordpress-mobile/gutenberg-mobile#1507
Handling spaces with Lists
From wordpress-mobile/gutenberg-mobile#885
Two spaces + Letter
Three spaces
Letter + Three Spaces
Undo test that causes the selection to exceed the record length
This scenario is why I cannot remove the record length check here.
Checklist: