[wp-trac] [WordPress Trac] #41394: Text widget: Rename legacy mode to visual mode and improve back-compat for widget_text filters (was: Application of widget_text filters passed unexpected $instance data)

[WordPress Trac]

#41394: Text widget: Rename legacy mode to visual mode and improve back-compat for
widget_text filters
-------------------------------------+--------------------------
 Reporter:  westonruter              |       Owner:  westonruter
     Type:  defect (bug)             |      Status:  accepted
 Priority:  normal                   |   Milestone:  4.8.1
Component:  Widgets                  |     Version:  trunk
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+--------------------------
Description changed by westonruter:

Old description:

> 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:
>
> {{{#!json
> {
>     "title": "The Title",
>     "text": "Hello World",
>     "filter": false
> }
> }}}
>
> When instead the Custom HTML widget instances look like:
>
> {{{#!json
> {
>     "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.
>
> This is also actually the case for existing `widget_text` filters for the
> Text widget itself. In 4.8 the `filter` param was set to a fixed
> `content` but if any filters do any checking of `$instance['filter'] ===
> true` then these checks will fail. Now that there is a boolean `legacy`
> instance property, we can eliminate the `content` value for the `filter`
> property and just always set it to `true`.

New description:

 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 filter`widget_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:

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

 When instead the Custom HTML widget instances look like:

 {{{#!json
 {
     "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.

--

--
Ticket URL: <https://core.trac.wordpress.org/ticket/41394#comment:8>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform