Make WordPress Core

Opened 3 weeks ago

Closed 2 weeks ago

Last modified 2 weeks ago

#61385 closed defect (bug) (fixed)

Revert using regex in block bindings HTML replacement logic

Reported by: santosguillamot's profile santosguillamot Owned by: audrasjb's profile audrasjb
Milestone: 6.6 Priority: high
Severity: normal Version: trunk
Component: HTML API Keywords: has-patch commit has-unit-tests
Focuses: Cc:

Description

Revert part of the changes made in this commit: https://core.trac.wordpress.org/changeset/58298

The idea of that code was to simplify the logic and potentially support more attributes in bindings, but it is causing other problems like this issue: https://github.com/WordPress/gutenberg/issues/62347#top

Using regex seems unreliable, so I believe it is safer to revert these changes and do the refactoring once the HTML API supports CSS selectors and provides a way to set inner content.

Change History (8)

This ticket was mentioned in PR #6744 on WordPress/wordpress-develop by @santosguillamot.


3 weeks ago
#1

## What?
Revert part of the changes made in this pull request to avoid using regex that can cause potential bugs like this one.

## Why?
The idea of that pull request was to simplify the logic and potentially support more attributes in bindings, but it is causing other problems like the mentioned issue. I believe it is safer to revert these changes and do the refactoring once the HTML API supports CSS selectors and provides a way to set inner content.

## How?
Just reverting the changes made to the previous implementation, which seemed to be working fine.

## Testing Instructions
Follow the testing instructions of https://github.com/WordPress/gutenberg/issues/62347#top and check that the issue is solved.

Trac ticket: https://core.trac.wordpress.org/ticket/61385

#2 @audrasjb
3 weeks ago

  • Component changed from General to HTML API
  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 6.6
  • Priority changed from normal to high

#3 @hmbashar
3 weeks ago

  • Keywords needs-testing added

#4 @gziolo
3 weeks ago

This patch brings back the version that we shipped in WordPress 6.5 and adds a unit test to cover the regression we experienced. It should be good to go.

#5 @audrasjb
2 weeks ago

  • Keywords commit added; 2nd-opinion needs-testing removed
  • Owner set to audrasjb
  • Status changed from new to reviewing

The patch works fine on my side. Thanks for bringing this solution and for adding a new unit test for the issue!

Self assigning for final review before committing this follow-up to changeset [58298].

#6 @audrasjb
2 weeks ago

  • Keywords has-unit-tests added

#7 @audrasjb
2 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 58398:

HTML API: Revert using regex in block bindings HTML replacement logic.

This changeset reverts part of the changes made in [58298] to avoid using regex that can cause potential bugs. It is indeed safer to revert these changes for now and do the refactoring once the HTML API supports CSS selectors and provides a way to set inner content.

It also adds a unit test to cover the regression experienced in https://github.com/WordPress/gutenberg/issues/62347.

Follow-up to [58298].

Props santosguillamot, gziolo.
Fixes #61385.
See #61351.

@audrasjb commented on PR #6744:


2 weeks ago
#8

Committed in https://core.trac.wordpress.org/changeset/58398
Also added @talldan missed props manually using the Core Props tool on Make/Core :)

Note: See TracTickets for help on using tickets.