Layout: Invalid CSS for nested fullwidth layouts with zeroed-out custom padding

[andrewserong]

An invalid CSS issue occurs when applying a padding rule of 0 to a full width group block, inside a full width group.

This creates an unwanted horizontal scroll, as the nested fullwidth value is not properly negated.

The fix is to treat 0 padding values as 0px in the Layout block support, so that the calc() rule applies correctly. This has been fixed in Gutenberg in ​https://github.com/WordPress/gutenberg/pull/63436, and needs to be backported to core, ideally in WP 6.6.1.

The issue is further described in this Gutenberg issue: ​https://github.com/WordPress/gutenberg/issues/63432

Backport ​https://github.com/WordPress/gutenberg/pull/63436 to core.

In the Layout block support, handle 0 values for padding as 0px in calc() rules. This resolves a bug for nested fullwidth layouts when zero padding is applied. Due to how calc() works, without supplying the unit, the rule will not work, resulting in a horizontal scrollbar. For more information see ​the original issue in the Gutenberg repo, or ​the fix.

### Testing instructions

1. Open a post or page.
2. Open the code editor within the top editor options panel.
3. Copy and paste ​this gist, which configures the conditions properly (nested align full blocks, with padding zeroed out on the parent).
4. See no horizontal scroll in the site front end.

Note: the issue will still be present in the post editor until the JS packages that include the JS changes from ​https://github.com/WordPress/gutenberg/pull/63436 are included in core.

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

Thanks @andrewserong, The changes look good to me.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/7036

Would be great to get a test report(s) to validate the before (reproduce the issue) and after applying the patch (to validate the gutenberg-merge resolves it).

Testing Instructions from the PR:

1. Open a post or page.
2. Open the code editor within the top editor options panel.
3. Copy and paste ​this gist, which configures the conditions properly (nested align full blocks, with padding zeroed out on the parent).
4. See no horizontal scroll in the site front end.

[noisysocks]

In 58750:

Block Themes: Fix invalid css for nested fullwidth layouts with zero padding applied

In the Layout block support, handle 0 values for padding as 0px in calc()
rules. This resolves a bug for nested fullwidth layouts when zero padding is
applied. Due to how calc() works, without supplying the unit, the rule will not
work, resulting in a horizontal scrollbar.

Backports the PHP changes in ​https://github.com/WordPress/gutenberg/pull/63436.

Fixes #61656.
Props andrewserong, mukesh27, aaronrobertshaw.

[noisysocks]

Re-opening to request backport of r58750 to the 6.6 branch.

Committed in r58750 and re-opened https://core.trac.wordpress.org/ticket/61656 to request backport to the 6.6 branch.

[hellofromTonya]

Test r58750 on 6.6 branch: shows not resolved in the editor.

[hellofromTonya]

Test Report

Patch tested: Tested [58750] on the 6.6 branch.

Environment

• OS: macOS
• Web Server: nginx
• PHP: 8.3
• WordPress: 6.6.1-alpha-58737-src
• Browser: Firefox and Edge
• Theme: Twenty Twenty-Four
• Active Plugins: None

Steps

1. Created a new post.
2. Opened the code editor within the top editor options panel.
3. Copied and pasted ​this gist, which configures the conditions properly (nested align full blocks, with padding zeroed out on the parent).
4. Observed behavior.

Actual Results

In the frontend:

• ✅ Issue is resolved.

In the editor:

• ❌ Issue is not resolved. See it in action in r58750-on-6.6branch.gif​.

[hellofromTonya]

Test Report

Patch tested: Tested [58750] with 6.6.0 and Gutenberg 18.8.0.

Environment

• OS: macOS
• Web Server: nginx
• PHP: 8.3
• WordPress: 6.6.0
• Browser: Firefox and Edge
• Theme: Twenty Twenty-Four
• Active Plugins: Gutenberg 18.8.0

Steps

1. Created a new post.
2. Opened the code editor within the top editor options panel.
3. Copied and pasted ​this gist, which configures the conditions properly (nested align full blocks, with padding zeroed out on the parent).
4. Observed behavior.

Actual Results

In the frontend:

• ✅ Issue is resolved.

In the editor:

• ✅ Issue is resolved.

[hellofromTonya]

Without the Gutenberg plugin activated, this fix resolves the issue on the frontend, but not within the editor ❌. See it in action r58750-on-6.6branch.gif​.

I tested it in the following ways with r58750 applied:

• on the 6.6 branch without the Gutenberg plugin activated.
• with 6.6.0 (the released version) and Gutenberg 18.8.0 activated.

Wondering if there's a package update or other change that is also needed.

Can retest after the package updates are committed and backported to the 6.6 branch.

But if not resolved after the package updates, thinking it's not ready for backporting to the 6.6 branch.

cc @ellatrix @noisysocks @andrewserong @vcanales

[hellofromTonya]

Oh wait, (facepalm) I see now. There is a ​package change in the Gutenberg PR fix. So this fix is dependent upon the package updates. Aha.

Will retest after the package updates.

[hellofromTonya]

In 58757:

Editor: Update packages for 6.6.1.

Bugfixes included:

• ​Elements: Avoid specificity bump for top-level element-only selectors.
• ​Navigation block: Allow themes to override block library text-decoration rule.
• ​Fix invalid css for nested fullwidth layouts with zero padding applied.
• ​Prevent empty void at the bottom of editor when block directory results are present.
• ​Pattern overrides: Ensure "Reset" button always shows as last item and with border.
• ​Global Styles: Disable "Reset styles" button when there are no changes.
• ​Fix: Removed shuffle button when only 1 pattern is present.
• ​fix: wp icon focus issue.
• ​useBlockElement: return null until ref callback has time to clean up the old element.

Props ellatrix.
Fixes #61692.
See #61660, #61630, #61656.

[hellofromTonya]

In 58760:

Editor: Update packages for 6.6.1.

Bugfixes included:

• ​Elements: Avoid specificity bump for top-level element-only selectors.
• ​Navigation block: Allow themes to override block library text-decoration rule.
• ​Fix invalid css for nested fullwidth layouts with zero padding applied.
• ​Prevent empty void at the bottom of editor when block directory results are present.
• ​Pattern overrides: Ensure "Reset" button always shows as last item and with border.
• ​Global Styles: Disable "Reset styles" button when there are no changes.
• ​Fix: Removed shuffle button when only 1 pattern is present.
• ​fix: wp icon focus issue.
• ​useBlockElement: return null until ref callback has time to clean up the old element.

Reviewed by spacedmonkey.
Merges [58757] to the 6.6 branch.

Props ellatrix.
Fixes #61692.
See #61660, #61630, #61656.

[hellofromTonya]

(Re)Test Report

This is a retest of comment:10 now that the packages are updated.

Patch tested: Tested [58750] on the 6.6 branch.

Environment

• OS: macOS
• Web Server: nginx
• PHP: 8.3
• WordPress: 6.6.1-alpha-58737-src
• Browser: Firefox and Edge
• Theme: Twenty Twenty-Four
• Active Plugins: None

Steps

1. Created a new post.
2. Opened the code editor within the top editor options panel.
3. Copied and pasted ​this gist, which configures the conditions properly (nested align full blocks, with padding zeroed out on the parent).
4. Observed behavior.

Actual Results

In the frontend:

• ✅ Issue is resolved.

In the editor:

• ✅ Issue is resolved.

[hellofromTonya]

[58750] LGTM for backport to the 6.6 branch.

Committing it now.

[hellofromTonya]

In 58761:

Block Themes: Fix invalid css for nested fullwidth layouts with zero padding applied

In the Layout block support, handle 0 values for padding as 0px in calc() rules. This resolves a bug for nested fullwidth layouts when zero padding is applied. Due to how calc() works, without supplying the unit, the rule will not
work, resulting in a horizontal scrollbar.

Ref: PHP changes from ​https://github.com/WordPress/gutenberg/pull/63436.

Reviewed by hellofromTonya.
Merges [58750] to the 6.6 branch.

Fixes #61656.
Props andrewserong, mukesh27, aaronrobertshaw.

Trac ticket: Core-61656

## Status

I'm nervous sharing this because it's full of flaws, needs extensive review, and documented test cases.

### Earlier versions

• First draft (ba2c8896f08ee36d3c61a500a86422b736a00923)
  • By implementing the rules at the top of Tree Construction/Dispatch, we don't need $skip_next_foreign_content_processing, because we pop nodes off of the stack of open elements until we're back at an html element, or an HTML integration point, or a MathML integration point. Returning to step() will choose _insertion mode_ instead of _foreign content_ and prevent an infinite loop.

### Notes

• There's an interplay we have to figure out with casing of tag names. For all of our comparison functions we check the upper-case variants, but there's value in reporting the foreign content tag names as lower-cased or as mixed case.
• There are some SVG tag names that are supposed to report in mixed case. For both SVG and MathML there are attributes which are supposed to report in mixed case. Unlike my first assumption, however, they still all parse with ASCII case insensitivity. That is, <circle clippath=1 clipPath=2 CLIPPATH=3 cliPPath=4> has a single attribute named clipPath and whose value is 1. This is convenient because we don't have to change the Tag Processor, but inconvenient when things are mostly based on lower-case attribute names.
• We probably need to repeal the idea that tag names are upper-case and add another bit to communicate tag name vs. doctype declaration. Maybe? Can we test that math:mi or math:MI is in the math namespace? There can be no HTML tag named math:mi (it would be MATH:MI).
• Could be handy to have something like base_class_get_tag() or private function comparison_tag_name() etc… to report an upper-case tag name, while preserving the case-variants required in foreign content to outside calls.
• Tracking the namespace seems a bit fishy. When we're in the Tag Processor we can know that if we encounter an SVG or MATH tag when we're in the html namespace, that it should change the namespace to svg or math, respectively, and lower-case the tag names. However, the role of integration points and parsing things in the insertion mode is still vague.
• Foreign content _is not_ an insertion mode. This is important when handling HTML tags inside HTML integration points. This is _very important_ for get_modifiable_text() which doesn't know if a text node inside foreign content is being processed _as foreign content_ or _in the insertion mode_, where it determines if NULL bytes should be replaced or removed.
• It's probably necessary to start over again with these insights and more in mind.

## Description

We should reliable detect foreign content and we need to do it in the Tag Processor, specifically because of the rules for CDATA sections. The HTML Processor needs this as well to determine if things like self-closing flags for HTML elements should be respected.

Unlocks:

• proper detection of CDATA
• parsing of SVG and MathML
• indicating if one should expect a closing tag or not

@sirreal @westonruter I've pushed some updates that incorporate trunk and a few other fixes. notably, I had the wrong code for adjusted_current_node().

This is very tricky. I'll share some cases that are problematic right now:

### <svg><a/>Text after svg:a self closing tag.
Errors with Cannot run adoption agency when "any other end tag" is required.

### <math><mtext><b>

### <svg></svg><![CDATA[ bogus comment ]]>

### <svg><foreignObject><![CDATA[ bogus comment ]]>

### <svg><title><div>

### <svg><title><rect><div>

### <svg><title><svg><div>

### <svg></br><foo>

### <math><mi><div></div></mi><mi>

### <math><mi><svg><foreignObject><div>

### <svg><foreignObject>\x00filler\x00text (null bytes)

I've made some progress on this fixing some of the issues mentioned above: ​https://github.com/WordPress/wordpress-develop/pull/6006#issuecomment-2262585496

Right now some self-closing tags in foreign content break. HTML like <svg><a/> seems to enter step_in_body, runs the adoption agency algorithm, and fails:

WP_HTML_Unsupported_Exception: Cannot run adoption agency when "any other end tag" is required. in /var/www/html/wp-includes/html-api/class-wp-html-processor.php:450
Stack trace:
#0 /var/www/html/wp-includes/html-api/class-wp-html-processor.php(5145): WP_HTML_Processor->bail('Cannot run adop...')
#1 /var/www/html/wp-includes/html-api/class-wp-html-processor.php(2423): WP_HTML_Processor->run_adoption_agency_algorithm()
#2 /var/www/html/wp-includes/html-api/class-wp-html-processor.php(4108): WP_HTML_Processor->step_in_body()
#3 /var/www/html/wp-includes/html-api/class-wp-html-processor.php(889): WP_HTML_Processor->step_in_foreign_content()
#4 /var/www/html/wp-includes/html-api/class-wp-html-processor.php(635): WP_HTML_Processor->step()}}}

Merged in [58867]
​https://github.com/wordpress/wordpress-develop/commit/de084d7d0e302027fb8f0a99bbeedf88e079efee

Trac ticket: Core-61656

HTML parsing rules at times differentiate character tokens that are all null bytes, all whitespace, or other content. This patch introduces a new function which may be used to classify text node sub-regions and lead to more efficient application of these parsing rules.

Further, when classified in this way, application code may skip some rules and decoding entirely, improving performance.

## Example script

<?php

require_once __DIR__ . '/src/wp-load.php';

$p = new class( "\x00\n\r&#x000000020;\n\x00\x00 \tStuff\x00\f<p>&#x13; &#9;\ntext<p>a \x00b<p>\x00\x00" ) extends WP_HTML_Tag_Processor {
        public function get_token_length() {
                $this->set_bookmark('here');
                return $this->bookmarks['here']->length;
        }
};

while ( $p->next_token() ) {
        if ( '#text' !== $p->get_token_name() ) {
                echo "\e[3;90mSkipping \e[0;2;32m{$p->get_token_name()}\e[m\n";
                continue;
        }

        $did_split = $p->subdivide_text_appropriately();
        $text = $p->get_modifiable_text();
        $text = str_replace( [ "\x00", "\t", "\f", "\r", "\n" ], [ '␤', '␉', '␌', '␍', '␤' ], $text );
        $after = $did_split ? " \e[3mafter splitting" : '';
        $length = $p->get_token_length();
        echo "\e[90mFound (\e[33m{$length}\e[90m) '\e[34m{$text}\e[90m'{$after}\e[m\n";
}

## html5lib tests

-Tests: 2433, Assertions: 4116, Skipped: 421.
+Tests: 2433, Assertions: 4112, Skipped: 425.

sorry for the late ticket creation, but I thought it was best to separate this as a feature enhancement. it's mostly an internal method, but since it has potential use to application code it can retain its own ticket.

Merged in [58970]
​https://github.com/wordpress/wordpress-develop/commit/95eb879c47c52413f33c3a62f3006262cc2b0062

Trac ticket: Core-61656.

The HTML specification indicates that an HTML tag with the name "IMAGE" should be renamed as "IMG" and handled as if it were an "IMG", but this only applies to elements in the HTML namespace.

In this patch the HTML Processor is updated to ensure that it doesn't remap the tag name when processing foreign content, such as SVG and MathML markup.

See #61576.

Merged in [59014]
​https://github.com/wordpress/wordpress-develop/commit/f7d9b1b12b67d328379965468145abf27316175c