Directory

Missing validation message to custom CSS input · Issue #56082 · WordPress/gutenberg · GitHub
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.

Sign up for GitHub

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

Jump to bottom

Missing validation message to custom CSS input #56082

Closed
afercia opened this issue Nov 13, 2023 · 5 comments · Fixed by #56093
Closed

Missing validation message to custom CSS input #56082

afercia opened this issue Nov 13, 2023 · 5 comments · Fixed by #56093
Assignees
@zaguiini
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Nov 13, 2023

Description

This appears to be a regression in trunk introduced in #49521

The validation messages for custom CSS introduced in #47132 no longer work.

The validation relied on the 'internal' CSS parser that was removed in #49521 in favor of postCSS.

While I understand the benefit of switching to postCSS, I'm a little surprised such an impactful change has been merged without double checking all the internal usages of the block editor utility transformStyles. For the future, I'd like to recommend to make such changes be reviewed by contributors who have a larger overview and knowledge of the editor features.
Cc @youknowriad @zaguiini @jsnajdr @strarsis

Step-by-step reproduction instructions

  • Go to the Site editor > Styles > More > Additional CSS
  • Add this in the Additional CSS textarea: body {background: red;}
  • Save.
  • Edit the textarea again and remove the closing bracket }.
  • Move focus away from the textarea.
  • Observe no validation message is shown.
  • Observe your browser dev tools console, you should see a message Uncaught CssSyntaxError: <css input>:1:1: Unclosed block
  • Test on WordPress 6.4 and repeat the steps above.
  • Observe that a red 'i" icon appears at the bottom right of the textarea.
  • Observe that hovering the icon, a tooltip appears with an error message.
  • Observe your browser dev tools console, you should see a message like Error while traversing the CSS: Error: undefined:1:22: missing '}'

Screenshots from WordPress 6.4 (expected behavior):

Screenshot 2023-11-13 at 15 24 33

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes
@afercia afercia added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release [Package] Block editor /packages/block-editor [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Nov 13, 2023
@afercia
Copy link
Contributor Author

afercia commented Nov 13, 2023

Aside: The validation message as implemented in #47132 is not accessible.

  • No audible feedback is provided.
  • The icon is just an SVG icon wrapper in a focusable div element.
  • The focusable div element has no labeling and no ARIA role, it's just a silent tab stop.
  • It should not be a focusable div in the first place.

I disagree with the design feedback provided in #47132. Instead of adopting single-use 'ad hoc' design inspired to other tools (Codepen in this case), we should just reuste the editor components. They aim to be accessible and reusable, whilc 'ad hoc' designs defeat the purpose of usability, accessibility and maintenance. I'd aruge ad-hoc implementations also defeat the purpose of reusable components in the first place.

@afercia
Copy link
Contributor Author

afercia commented Nov 13, 2023

Note: Some E2E tests for the custom CSS validation messages would have been nice and would have allowed to catch this issue earlier.

@zaguiini
Copy link
Contributor

zaguiini commented Nov 13, 2023 •
edited

While I understand the benefit of switching to postCSS, I'm a little surprised such an impactful change has been merged without double checking all the internal usages of the block editor utility transformStyles.

It was tested, but it's hard to grasp all the places where it's being used. Indeed, there are most tests right now than before because of checking. Unfortunately it wasn't 100% checked, apparently.

One good action would be to introduce a new test case where invalid CSS is passed so that we can prevent this bug in the future.

Looking at the code, it looks like the fix would be to wrap the post CSS call with a try-catch block.

@zaguiini zaguiini mentioned this issue Nov 13, 2023
Merged
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Nov 13, 2023
@strarsis
Copy link
Contributor

Yes, this is a good opportunity for adding missing tests!

@Marc-pi Marc-pi mentioned this issue Nov 15, 2023
Closed
@Marc-pi
Copy link

Marc-pi commented Nov 15, 2023 •
edited

good catch, took me a while to spot the error, thanks to this issue !!!
see woocommerce/woocommerce-blocks#11730
indeed, i do confirm that the error highlighter is missing,
the codeMirror feature is indeed missing see this PR #53380

@arthur791004 arthur791004 mentioned this issue Nov 17, 2023
Closed
@afercia afercia mentioned this issue Nov 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zaguiini zaguiini

Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

PostCSS style transformation: fail gracefully instead of throwing an error WordPress/gutenberg
4 participants
@Marc-pi @afercia @strarsis @zaguiini