WordPress.org
Get WordPress

Make WordPress Core

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#41394 closed defect (bug) (fixed)

Text widget: Rename legacy mode to visual mode and improve back-compat for widget_text filters

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 4.8.1 Priority: normal
Severity: normal Version: 4.9
Component: Widgets Keywords: has-patch commit fixed-major
Focuses: Cc:

Description (last modified by westonruter)

When TinyMCE was introduced to the Text widget in #35243, the filter prop was overloaded to take a truthy content string value, in addition to what it had formerly as a simple boolean. The filter=content was taken to mean that the content filters should apply (widget_text_content), in addition to just wpautop. When filter=true then just wpautop would apply, and when filter=false then no wpautop would apply. The filter=content prop was then also used as a signal for whether or not the visual editor should be loaded.

Problem: Overloading the filter boolean to take this tertiary state, however, could cause problems for plugins that look explicitly for a filter=true value in existing plugins that filterwidget_text.

In #40951 there was a legacy boolean prop added to modified Text widgets, to indicate that the visual editor should not be loaded: when legacy=true then the new TinyMCE editor would be displayed. This new property was added since the filter property couldn't be overloaded yet further.

Nevertheless, the overloading of the filter property to be boolean and also a string of content could be undone, and the legacy property added in #40951 could change from being a simple single-value flag to instead be a boolean visual property. A visual=true property would indicate that TinyMCE should be loaded, whereas visual=false would indicate that it should not be loaded. If visual=null when the form is loaded, then the is_legacy_instance logic is used to set the initial value.

Furthermore, the Custom HTML widget (#40907) applies the widget_text filters on its content to ensure that when a user moves HTML from a Text widget over to a Custom HTML widget, it will get all of the same filters applied. See [41086]. There are still widget_text_content and widget_custom_html_content filters that apply to the Text widget and Custom HTML widget respectively, but they share this same widget_text filter that has been around since WP 2.3.0.

When the Custom HTML widget applies the widget_text filters, it is supplying the Custom HTML widget's $instance as the filter's second argument. This could cause problems for plugins that filter widget_text because they may expect a widget's instance data to look like:

{
    "title": "The Title",
    "text": "Hello World",
    "filter": false
}

When instead the Custom HTML widget instances look like:

{
    "title": "The Title",
    "content": "Hello World"
}

So in order to preserve backwards compatibility for plugins that look at the $instance param, the instance data needs to be transformed from the Custom HTML instance schema over to the Text widget instance schema.

Change History (19)

@westonruter
9 years ago

#1 @westonruter
9 years ago

  • Keywords has-patch added
  • Owner set to westonruter
  • Status changed from new to accepted

#2 @westonruter
9 years ago

  • Description modified (diff)

This ticket was mentioned in ​Slack in #core-customize by westonruter. ​View the logs.
9 years ago

#4 @obenland
9 years ago 

<?php
// Prepare instance data that looks like a Text widget. 
$simulated_text_widget_instance = wp_parse_args( array(
        'text'   => $instance['content'],
        'filter' => false, // Because wpautop is not applied. 
), $instance );

Might be a little easier to grok. It shouldn't matter if content remains or not.

#5 @westonruter
9 years ago

  • Description modified (diff)
  • Summary changed from Custom HTML widget applies widget_text filters with unexpected $instance data to Application of widget_text filters passed unexpected $instance data

I'm expanding the scope here to also account for widget_text filters applying when plugins will expect $instance['filter'] to be a boolean, not a boolean with a possible string value of content.

@westonruter
8 years ago

​https://github.com/xwp/wordpress-develop/pull/241/files/2d1362d..6e8ef5d

#7 @westonruter
8 years ago

  • Keywords needs-testing added

41394.1.diff​ needs testing.

@westonruter
8 years ago

​https://github.com/xwp/wordpress-develop/pull/241/commits/e30ab419d811d3d5bcf3776f81559a3be3db7a60

#8 @westonruter
8 years ago

  • Description modified (diff)
  • Summary changed from Application of widget_text filters passed unexpected $instance data to Text widget: Rename legacy mode to visual mode and improve back-compat for widget_text filters

This ticket was mentioned in ​Slack in #core-customize by westonruter. ​View the logs.
8 years ago

This ticket was mentioned in ​Slack in #core-customize by westonruter. ​View the logs.
8 years ago

#11 @westonruter
8 years ago

  • Keywords commit added; needs-testing removed

Thanks to @timmydcrawford for testing. I'll commit this in the next couple hours unless someone identifies something.

#12 @westonruter
8 years ago

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

In 41132:

Widgets: Rename Text widget's legacy mode to non-visual mode, restore boolean filter prop, and improve compatibility for widget_text filters applied in Custom HTML widget.

Props westonruter, obenland, timmydcrawford for testing.
Amends [41050].
See #35243, #40951, #40907.
Fixes #41394.

#13 @westonruter
8 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for 4.8.1

#14 @westonruter
8 years ago

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

In 41133:

Widgets: Rename Text widget's legacy mode to non-visual mode, restore boolean filter prop, and improve compatibility for widget_text filters applied in Custom HTML widget.

Merges [41132] onto 4.8 branch.
Amends [41050].
Props westonruter, obenland, timmydcrawford for testing.
See #35243, #40951, #40907.
Fixes #41394 for 4.8.1.

#15 @westonruter
8 years ago

In 41134:

Widgets: Improve notice in Text widget's legacy mode.

Also fixes phpunit test which broke in [41132].

Props melchoyce, westonruter, gk.loveweb.
See #41394, #35243, #40951, #40907.
Fixes #41386.

#16 @westonruter
8 years ago

In 41135:

Widgets: Improve notice in Text widget's legacy mode.

Retains 'new' in notice on 4.8 branch, in contrast with [41134]. Also fixes phpunit test which broke in [41132].

Merges [41134] onto 4.8 branch.
Props melchoyce, westonruter.
See #41394, #35243, #40951, #40907.
Fixes #41386 for 4.8.1.

Note: See TracTickets for help on using tickets.