Custom HTML Widget uses widget_text twice in markup

[dreamwhisper]

In https://core.trac.wordpress.org/changeset/41117, classes were added to the Custom HTML Widget to apply the same styling as the Text Widget in themes.

However, there's an extra widget_text class that isn't in the Text Widget.

Example markup:

Custom HTML widget - The widget_text is on the section and the widget-wrap div.

<section id="custom_html-2" class="widget_text widget widget_custom_html">
<div class="widget_text widget-wrap">
<h3 class="widgettitle widget-title">Custom HTML</h3>
<div class="textwidget custom-html-widget">
Custom HTML Content
</div>
</div>
</section>

Text Widget - The widget-wrap div has only that class.

<section id="text-2" class="widget widget_text">
<div class="widget-wrap">
<h3 class="widgettitle widget-title">Text Widget</h3>
<div class="textwidget">
<p>Text Widget Content</p>
</div>
</div>
</section>

As a result, any theme that has styled widget_text may have unintended styling issues.

[dreamwhisper]

For additional information, this only happens if a theme alters the before_widget attribute to include an element with a class attribute. This is due to the regex in https://core.trac.wordpress.org/changeset/41117

<?php
register_sidebar( array(
                'name' => __( 'Primary Widget Area', 'twentyten' ),
                'id' => 'primary-widget-area',
                'description' => __( 'Add widgets here to appear in your sidebar.', 'twentyten' ),
                'before_widget' => '<div id="%1$s" class="widget-container %2$s"><div class="widget-wrap">',
                'after_widget' => '</div></div>',
                'before_title' => '<h3 class="widget-title">',
                'after_title' => '</h3>',
        ) );

[alexvorn2]

I have the same problem!
I can add custom class name "widget_text" for Custom HTML widget in the PHP widget name class, or we can change how it CSS class name is added to the string...
Like checking for ID of the widget...

Example:

<?php
$widget_box_array['before_widget'] = str_replace( 'id="' . $widget_id . '" class="', 'id="' . $widget_id . '" class="widget_text ', $widget_box_array['before_widget'] );

[westonruter]

The inclusion of the widget_text class on the Custom HTML widget was intentional. It is for backwards-compatibility, when users switch from the Text widget to the Custom HTML widget but want to retain styles. If you want to target the Text widget without the Custom HTML widget, use the CSS selector: .widget_text:not(.widget_custom_html). See this dev note: ​https://make.wordpress.org/core/2017/08/01/fixes-to-text-widget-and-introduction-of-custom-html-widget-in-4-8-1/

Is this not what is being reported here?

The Custom HTML widget is injecting the widget_text class name via:

$args['before_widget'] = preg_replace( '/(?<=\sclass=["\'])/', 'widget_text ', $args['before_widget'] );

Is this causing a problem?

[dreamwhisper]

I agree that it should be included for the Custom HTML widget. However, I think it should be included in a way that it only adds the widget_text class to the section in my example.

What's happening now is the regex is catching everything in the before_widget markup.

Thus using <div id="%1$s" class="widget-container %2$s"><div class="widget-wrap"> for before_widget will result in the markup below for the Custom HTML widget, but not the Text widget:

<section id="custom_html-2" class="widget_text widget widget_custom_html">
<div class="widget_text widget-wrap">

I don't think the intent was to add the class twice for both the section and the div.

[alexvorn2]

So is my code good? Or we need only to use REGEX algorithm?

$widget_id is the string name of the widget

So the code is this:

$args['before_widget'] = str_replace( 'id="' . $widget_id . '" class="', 'id="' . $widget_id . '" class="widget_text ', $args['before_widget'] );

It will replace only once because it will search for widget ID too, and the first div will be changed...

Or If someone is good at REGEX to make this code replace only once:

$args['before_widget'] = preg_replace( '/(?<=\sclass=["\'])/', 'widget_text ', $args['before_widget'] );

[alexvorn2]

I found we can add the 4th parameter "1" in the preg_replace function, that will replace only once, so it will look like this:

$args['before_widget'] = preg_replace( '/(?<=\sclass=["\'])/', 'widget_text ', $args['before_widget'], 1 );

But what if we have wrapper div elements? ... that will appear before not only after.

[dreamwhisper]

Is there any information needed to further describe the bug in this issue?

[westonruter]

Replying to alexvorn2:

I found we can add the 4th parameter "1" in the preg_replace function, that will replace only once, so it will look like this:

$args['before_widget'] = preg_replace( '/(?<=\sclass=["\'])/', 'widget_text ', $args['before_widget'], 1 );

But what if we have wrapper div elements? ... that will appear before not only after.

I think you have the right approach here. The wrapper element should have a class added to it, or else things won't work as expected in the Customizer preview: ​https://github.com/WordPress/wordpress-develop/blob/7c2f975/src/wp-includes/js/customize-preview-widgets.js#L609-L619

The only hardening that may be warranted is to check if the before_widget actually starts with an element that has a class attribute in it. For example:

<?php
if ( preg_match( '#^<[^>]+class=[^>]+>#', $args['before_widget'] ) ) {
    $args['before_widget'] = preg_replace( '/(?<=\sclass=["\'])/', 'widget_text ', $args['before_widget'], 1 );
}

But that's probably somewhat overkill.

[pento]

This ticket needs further investigation and a decision.