Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mobile] Native Bridge: Fix insert blocks not handling raw string properly in unsupported block editor #47472

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

renatoagds
Copy link
Contributor

What?

Fixing insertBlocks at the react-native-bridge call not dealing with strings that contain complex chars (like unicode).

Why?

Attributes could contain HTML code, which is serialized with special chars.

Example:

<!-- wp:videopress/video {"title":"174023215_5408417369200486_4341486698209593172_n-mp4","description":"","id":29,"guid":"YzEZpv1G","src":"https://videopress.com/v/YzEZpv1G","cacheHtml":"\u003ciframe title=\u0022VideoPress Video Player\u0022 aria-label='VideoPress Video Player' width='600' height='750' src='https://videopress.com/embed/YzEZpv1G?cover=1\u0026amp;preloadContent=metadata\u0026amp;useAverageColor=1\u0026amp;hd=1' frameborder='0' allowfullscreen data-resize-to-parent=\u0022true\u0022 allow='clipboard-write'\u003e\u003c/iframe\u003e\u003cscript src='https://v0.wordpress.com/js/next/videopress-iframe.js?m=1658470809'\u003e\u003c/script\u003e","videoRatio":125,"privacySetting":2,"allowDownload":false,"rating":"G","isPrivate":false,"className":"\\\u0022wp-block-videopress-video"} -->
  <figure class="wp-block-videopress-video wp-block-jetpack-videopress jetpack-videopress-player \&quot;wp-block-videopress-video">
    <div class="jetpack-videopress-player__wrapper">
https://videopress.com/v/YzEZpv1G?resizeToParent=true&amp;cover=true&amp;preloadContent=metadata&amp;useAverageColor=true
    </div>
  </figure>
<!-- /wp:videopress/video -->

In the current way, it parses the \u and other chars, which breaks the block.

How?

Inserting String.raw on insertBlocks call.

Testing Instructions

  1. Run WordPress Mobile (iOs or Android) with gutenberg-mobile running locally.
  2. Open a post that contains an unsupported block that has complex attributes (new VideoPress block for example).
  3. Click on ? and try to edit it on the web editor.
  4. It should work.

Screenshots or screencast

Before After
CleanShot.2023-01-26.at.18.49.38.mp4
CleanShot.2023-01-26.at.18.40.55.mp4
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 26, 2023
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @renatoagds! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@fluiddot fluiddot added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Jan 27, 2023
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @renatoagds for fixing this issue 🙇 !

I tested this change in physical devices and the simulator/emulator and I'm still experiencing the issue:

  • iPhone 11 (iOS 15.4.1) ❌
  • Samsung Galaxy S20 FE 5G (Android 13) ❌
  • iPhone 14 - Simulator ✅ (Not sure why only in the iOS simulator works 🤔 )
  • Pixel 3 API 28 emulator ❌

I'll keep investigating in case I can provide further information that helps us to address it. In the meantime, @renatoagds did you test this on a physical device? Thanks!

@fluiddot
Copy link
Contributor

Following my previous comment, I noticed that I'm experiencing a different error. In my case, it's not an invalid markup format but the block missing.

Error extracted from Before video capture:
Screenshot 2023-01-27 at 16 42 59

The error I'm experiencing:
Untitled

@renatoagds what type of site you used for test the changes?

@renatoagds
Copy link
Contributor Author

@fluiddot I tested in an Atomic site with VideoPress plugin installed. If you're trying on a simple site, you must be proxied since this block is still under the beta flag. Are you trying on simple?

@renatoagds
Copy link
Contributor Author

@fluiddot I sent an invitation for you to test blog I'm using (atomicv6).

@fluiddot
Copy link
Contributor

Ah, ok, thanks @renatoagds 🙇. I was testing the block now on different sites and noticed the difference. I'll re-test the PR as soon as possible 👍 .

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now using the Atomic site you provided @renatoagds I managed to confirm the fix works as expected, thanks! The PR LGTM 🎊 !

Tested on Samsung Galaxy S20 FE 5G and iPhone 11 (iOS 15.4.1).

@fluiddot fluiddot changed the title Native Bridge: Fix insert blocks not handling raw string properly in unsupported block editor Jan 27, 2023
@fluiddot fluiddot merged commit 6a64cdf into WordPress:trunk Jan 27, 2023
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Jan 27, 2023
fluiddot pushed a commit that referenced this pull request Jan 27, 2023
fluiddot added a commit that referenced this pull request Jan 31, 2023
* Release script: Update react-native-editor version to 1.87.0

* Release script: Update with changes from 'npm run core preios'

* Mobile - Update changelog

* Release script: Update react-native-editor version to 1.87.1

* Release script: Update with changes from 'npm run core preios'

* [RNMobile] Remove unnecessary negative margin around empty gallery block (#47086)

This PR removes some unwanted negative margin space from empty gallery blocks on native.

* [Mobile] - RichText - Parse CSS values and avoid setting undefined ones (#47080)

* parseCssUnitToPx - Fix issue with decimals in math expressions

* Mobile - RichText - Parse CSS values from line height values and avoid undefined or NaN values being sent to the native TextInput

* Mobile - Update Changelog

* Release script: Update react-native-editor version to 1.87.2

* Release script: Update with changes from 'npm run core preios'

* Add boolean contentStyle and clientId check to Column Edit InnerBlocks

* Mobile - Column block - Move parentWidth to a const variable

* docs: Update changelog

* [Mobile] - Line-height and font-size regression fixes (#47284)

* Revert "[Mobile] - RichText - Parse CSS values and avoid setting undefined ones (#47080)"

This reverts commit 2d44f06.

* parseCssUnitToPx - Fix issue with decimals in math expressions

* Mobile - Avoid passing NaN line height values to Aztec. It also disables passing font size and line height values on Android for pre elements.

* Mobile - Preformatted - Update snapshot to remove the fontSize value as it will be set by Aztec for Android

* Update react-native-editor changelog

* Release script: Update react-native-editor version to 1.87.3

* Release script: Update with changes from 'npm run core preios'

* [Mobile] Native Bridge: Fix insert blocks not handling raw string properly in unsupported block editor (#47472)

* Update react-native-editor changelog

---------

Co-authored-by: Gerardo <gerardo.pacheco@automattic.com>
Co-authored-by: Siobhan Bamber <siobhan@automattic.com>
Co-authored-by: David Calhoun <438664+dcalhoun@users.noreply.github.com>
Co-authored-by: Derek Blank <derekpblank@gmail.com>
Co-authored-by: Renato Augusto Gama dos Santos <renato_0603@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended
2 participants