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

Classic block: Optional chain on possibly null editor #25163

Merged
merged 2 commits into from
Sep 9, 2020

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Sep 8, 2020

Description

We've observed errors where getContent is invoked on null on this
line. Use optional chaining to prevent this error from happening.

const currentContent = editor.getContent();

I don't know how to reproduce this consistently, but I have observed the error regularly on a site of mine and this change fixes the error.

On my site, the error is triggered by adding a Classic block with some content, saving the post, reloading the post in the editor then selecting the Classic block:

Introduced in #23408.

How has this been tested?

Rebuilding the plugin zip with this change and uploading it to my site.

Screenshots

Screen Shot on 2020-09-08 at 10-03-40

Types of changes

Bug fix: Fix an issue where the Classic block may throw an error when loaded in a post.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

We've observed errors where `getContent` is invoked on `null` on this
line. Use optional chaining to prevent this error from happening.
@github-actions
Copy link

github-actions bot commented Sep 8, 2020

Size Change: +53 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/a11y/index.js 1.14 kB +1 B
build/block-directory/index.js 8.5 kB -1 B
build/block-editor/index.js 128 kB -196 B (0%)
build/block-editor/style-rtl.css 11.1 kB +3 B (0%)
build/block-editor/style.css 11.1 kB +3 B (0%)
build/block-library/editor-rtl.css 8.68 kB +43 B (0%)
build/block-library/editor.css 8.68 kB +44 B (0%)
build/block-library/index.js 139 kB +282 B (0%)
build/blocks/index.js 47.7 kB +1 B
build/components/index.js 200 kB -61 B (0%)
build/data/index.js 8.54 kB -2 B (0%)
build/edit-post/index.js 305 kB +3 B (0%)
build/edit-site/index.js 19.3 kB -65 B (0%)
build/edit-widgets/index.js 12.1 kB +2 B (0%)
build/editor/index.js 45.6 kB -5 B (0%)
build/format-library/index.js 7.72 kB +2 B (0%)
build/redux-routine/index.js 2.85 kB -2 B (0%)
build/url/index.js 4.06 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.41 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-library/style-rtl.css 7.59 kB 0 B
build/block-library/style.css 7.58 kB 0 B
build/block-library/theme-rtl.css 754 B 0 B
build/block-library/theme.css 754 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.48 kB 0 B
build/edit-navigation/index.js 11.7 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/style-rtl.css 6.26 kB 0 B
build/edit-post/style.css 6.25 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.46 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 kB 0 B
build/element/index.js 4.64 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

This can still happen if you load and you click really fast the Classic block toolbar, before the init happens. So it can be an extra safeguard and handles the above case. Remove the second change and let's merge this! 💯

packages/block-library/src/classic/edit.js Outdated Show resolved Hide resolved
Co-authored-by: Nik Tsekouras <[email protected]>
@sirreal sirreal marked this pull request as ready for review September 9, 2020 12:00
@sirreal sirreal self-assigned this Sep 9, 2020
@sirreal sirreal added [Block] Classic Affects the Classic Editor Block [Type] Bug An existing feature does not function as intended labels Sep 9, 2020
@sirreal
Copy link
Member Author

sirreal commented Sep 9, 2020

Thanks for the review, @ntsekouras. I've reverted the second optional chain, this should be ready.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants