Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#40951 closed defect (bug) (fixed)

New Text Widget - Switching Between Visual/Text Editor Strips Out Code

Reported by: dwrippe's profile dwrippe Owned by: westonruter's profile westonruter
Milestone: 4.8.1 Priority: normal
Severity: major Version: 4.8
Component: Widgets Keywords: needs-testing has-unit-tests has-patch fixed-major has-dev-note
Focuses: Cc:

Description

Just noticed the issue this morning, after upgrading to WP 4.8 on a few sites.

The new Text widget (which defaults to the Visual Editor) is occasionally stripping out existing code that was placed in the widget prior to the existence of the visual editor.

For example, if the following code is placed in the widget via the Text editor:

<ul>
<li><a href="#" class="location"></a>List Item 1</li>
<li><a href="#" class="location"></a>List Item 2</li>
</ul>

When the Widgets page is accessed after first updating, the Text widgets default to the Visual editor. This action, or switching between the two editors at any point, is stripping out the <a> link code (presumably because there isn't any text associated with it).

Considering the old functionality of the text widget, this may present wide-spread issues with people who have previously used Text Widgets to handle basic HTML code in a way that wasn't subject to the considerations of a Visual editor adjusting it (as it does in Pages/Posts).

Attachments (24)

text-widget-legacy-mode.png (87.3 KB) - added by westonruter 7 years ago.
Mock of Text widget in legacy mode
text-widget-legacy-mode-2.png (254.8 KB) - added by westonruter 7 years ago.
Second screenshot of Text widget in legacy mode
40951.0.diff (17.4 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235
40951.1.diff (17.4 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/commits/e20339bc92cfaca72789334ddedba7fb1e7d61d4
admin-pointer-customizer.png (114.4 KB) - added by westonruter 7 years ago.
Admin pointer shown upon clicking Text tab in Customizer
admin-pointer-widgets-screen.png (92.5 KB) - added by westonruter 7 years ago.
Admin pointer shown upon clicking Text tab in admin screen
40951.2.diff (21.2 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/e20339b..1b49d42
40951.3.diff (21.2 KB) - added by westonruter 7 years ago.
Refresh patch
40951.4.diff (21.8 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/d8cfffb..beb1c76
40951.5.diff (26.0 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/commits/6e6b53df1f5ccdcc009682dcc9166c88d8c2c56d
40951.6.diff (27.0 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/ae17fe2..af404a6
40951.7.diff (30.3 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/af404a6..55ea55c
widget-text-dismissible-notice-instead-of-pointer.png (124.0 KB) - added by westonruter 7 years ago.
Mock of how dismissible notice could be used instead of admin pointer
40951.8.diff (27.3 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/55ea55c..71c406ddc
pseudo-admin-pointer-on-widgets-admin-screen.png (116.4 KB) - added by westonruter 7 years ago.
pseudo-admin-pointer-in-customizer.png (96.1 KB) - added by westonruter 7 years ago.
40951.9.diff (29.1 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/commits/6e6c5501c5551f5fbb40293eabed62d85072088e
add-widget-link-auto-search-available-widgets-panel.png (173.4 KB) - added by westonruter 7 years ago.
microdata-retained-in-legacy-mode.png (252.4 KB) - added by westonruter 7 years ago.
40951.10.diff (31.4 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/6e6c550..d31f7ba
40951.11.diff (31.7 KB) - added by westonruter 7 years ago.
Use legacy mode when HTML comment is present: https://github.com/xwp/wordpress-develop/pull/235/commits/57a24f42e464b731299d77d8be50e4c4f0101766
40951.12.diff (32.1 KB) - added by westonruter 7 years ago.
Improve accessibility of pointers: https://github.com/xwp/wordpress-develop/pull/235/commits/2b7d10b087994b20dc205c1710c89e9d412189f7
40951.13.diff (2.3 KB) - added by westonruter 7 years ago.
https://github.com/xwp/wordpress-develop/pull/235/files/41b0538e20161f6cca85bb3dc87abdb581279a73..59949be0debee71d29150098a27a8cc335e062a1
custom-html-widget-shortcode-support.diff (1.2 KB) - added by westonruter 7 years ago.

Download all attachments as: .zip

Change History (201)

#1 @dwrippe
7 years ago

  • Severity changed from normal to minor

#2 @westonruter
7 years ago

@dwrippe thanks for the report. It seems you're right that TinyMCE is (rightly?) stripping out extraneous HTML. Do you have other examples where non-extraneous HTML is being stripped out?

#3 @dwrippe
7 years ago

@westonruter Playing with it some more, it only appears to happen with certain elements. For example, empty <a> and <span> elements disappear, while empty <div> elements do not. I haven't had time to run through the gamut of different HTML elements.

I don't really have a problem with the new functionality (there are ways around it, like dumping in text and using CSS to set the font-size to zero), but what concerns me is existing solutions. Say, for example, I have put a linked map in a widget space that is styled and displayed exclusively via CSS. If the code is something as simple as <a href="#" class="map"></a>, that is going to disappear the next time I (or a client) modifies and saves that Text widget.

I should point out that the Text widget is NOT modified unless you actually re-save the contents. I first discovered the initial example when trying to figure out how I'd linked the List items, because I didn't see <a> tags in the text widget. The links still worked on the front-end. Upon saving the Text widget, the links on the front-end stopped working, which alerted me to what had happened (if that makes sense).

#4 @dwrippe
7 years ago

Maybe the best way to handle the situation is just not to default the new Text Widget to the Visual Editor. Or at least not default existing Text Widgets to the new Visual editor (and only newly placed Text widgets would default to that). If the widget defaults to the Text Editor, the existing code (in theory) shouldn't be modified, and people would be more apt to notice content being stripped out when they manually flip between the two.

I would agree the TinyMCE is probably rightly stripping out extraneous tags (the same happens in the Post editor), but considering that for the last many, many years we could put HTML in a Text Widget and nothing would mess with it, this could potentially (and unknowingly) affect a lot of people.

Or maybe I've just been using the Text Widget incorrectly all these years. ;)

Last edited 7 years ago by dwrippe (previous) (diff)

#5 @westonruter
7 years ago

@azaozz Does any of this indicate regressions with TinyMCE?

@dwrippe For writing arbitrary HTML that isn't managed by TinyMCE, you're going to want to start using a new HTML Code widget (#40907) being worked on for 4.8.1.

#6 @dwrippe
7 years ago

@westonruter Looking forward to the new HTML Code widget. That sounds like a good solution.

While probably too late at this point, perhaps it would have made more sense to convert the old Text Widget into the new HTML Code widget, and release the Visual Editor Text Widget as the new standalone (in an effort to reduce conflicts with existing usage).

(In fact, I see that very point was brought up in #40907. C'est la vie!)

Last edited 7 years ago by dwrippe (previous) (diff)

#7 @westonruter
7 years ago

@dwrippe this suggestion was discussed prior to the 4.8 release. See long conversation before and after https://wordpress.slack.com/archives/C02RQBWTW/p1496265448106375

However, ultimately it was weighed that the Text widget should be updated to align with the post editor, and that a new dedicated widget should be introduced for allowing a user to add arbitrary allowed HTML (#40907).

#8 @dwrippe
7 years ago

@westonruter Yeah, I understand why. The bulk of WordPress users are not developers, and so updating the old Text widget to the new one makes sense for probably the larger population of WP users.

I would honestly be happy with a change in 4.8.1 to not make the Visual editor the default in Text widgets. That way, I can wait on upgrading sites, and when the the new HTML Code Widget is available, I can more easily go in and copy/paste existing HTML out of Text widgets and into the new HTML Code widget. As it stands, with the Visual Editor being the default, I have no opportunity for recovering extraneous HTML code that currently exists in Text widgets.

After developers have an opportunity to address the new functionality, the new Text widget could be set back to using the Visual editor as the default. But at least give us a chance to convert...

Last edited 7 years ago by dwrippe (previous) (diff)

#9 @westonruter
7 years ago

  • Keywords close added

Pending any other feedback that comes in over the next day or so, I think this should be closed in favor of #40907.

#10 @carasmo
7 years ago

Many users don't add a plugin for every little thing you want to add in a widget, so the <4.8 text widget was used to put in html for MailChimp forms or other little things and don't check the checkbox and go! Now with the new text widget, unless that html is completely minified, when you re-visit the text widget to edit that instance, the result is that the html has now added p tags around form elements and other stuff where it's not only a validation error in some cases, the layout is entirely messed up. This royally sucks. Anyway, for my purposes I forked the old text widget and named it Classic Text Widget. https://gist.github.com/carasmo/d29722a1ef7e1ebe29125661ed3690a2

#11 @carasmo
7 years ago

Made a plugin, no time to submit WordPress. I am using TGM to get it from GitHub. https://github.com/carasmo/classic-text-widget

I have TGM in the Genesis child theme which requires a theme specific plugin that privately updates, then I use TGM in the plugin to add required/recommended plugins for situations like this.

This ticket was mentioned in Slack in #forums by jeffr0. View the logs.


7 years ago

#13 @westonruter
7 years ago

  • Keywords needs-patch added; close removed

TinyMCE can be configured to prevent stripping empty elements. See https://wordpress.slack.com/archives/C02RQBWTW/p1496955151181007

This ticket was mentioned in Slack in #core by westonruter. View the logs.


7 years ago

#16 @westonruter
7 years ago

  • Milestone changed from Awaiting Review to 4.8.1

#17 @westonruter
7 years ago

I was chatting with @matveb about this dilemma and he suggests that we disable the new TinyMCE experience for the Text widget if previously the autop checkbox had been unchecked, that is if the instance has filter as false. When user adds a new Text widget and then selects the Text tab to paste in HTML, they can get an admin pointer as noted in #40907 to direct them to add an HTML Code widget instead.

The one hesitation I have about this solution is that the “Automatically add paragraphs” checkbox was unchecked by default. So that could mean that there are many users who have Text widgets which have plain text that would lack the new visual editor when they didn't have multiple paragraphs.

#18 @synavista
7 years ago

(this is @dwrippe - decided to setup a WordPress.org account with my corporate email rather than using my personal account)

@westonruter just caught up on the Slack convo regarding this. Thanks for the quick and devoted conversation to an issue that is affecting an unknown but possibly larger number of users than expected. Situations where 3rd parties have recommended using Text widgets and scripts to add forms (like MailChimp) is one that didn't cross my mind but will undoubtedly affect a small handful of our clients that use MailChimp and have forms embedded via Text widgets. As you said, it may not be proper usage, but it's what MailChimp (and probably others) have been recommending for years.

My 2 cents, for what it's worth - it does seem like the best solution thus far (per the options discussed in Slack, if I'm understanding them correctly), is to default to the Text Editor in the TinyMCE widget (which I assume would preserve existing code?), and include a note alerting users of the new HTML Code Widget, providing the opportunity to move existing HTML into the new HTML Code widget (or risk losing it by switching to the Visual editor). I believe that's what you are asking about here? https://core.trac.wordpress.org/ticket/40960#comment:1

I would agree that it doesn't really make sense to try and force the the new TinyMCE Text Widget to be an HTML editor, but folks should at least have the ability/opportunity to make the change to the "correct" usage.

Last edited 7 years ago by synavista (previous) (diff)

#19 @synavista
7 years ago

In 4.7.x, having the autop checkbox ticked doesn't really prevent people from embedding forms via <script> tags into the Text Editor, and extraneous inline elements such as empty <a> or <span> tags still flow through to the front end. At least, the MailChimp form I just tried in 4.7.x, pasted into a Text Widget with the autop checkbox ticked came through just fine on the front end, as did the example scenario from my initial ticket description.

That being said, I also have no problem pasting MailChimp form code into the new TinyMCE Text Widget and flipping between the Visual and Text editors, an issue @carasmo was seeing but I'm unable to replicate.

If the status of the autop checkbox was used to determine what editor in the TinyMCE Text Widget was displayed, users may still find themselves in a situation where they had unrecoverable content.

#20 @carasmo
7 years ago

It made a bunch of the credit card icons disappear (turn into a single &nbsp;) and <span class="fa fa-cc-discover fa-2x" aria-hidden="true"></span>. This below html for MailChimp, it displays as single line email input and submit button on the same line. When it's pasted into the new text widget, unless there are no line returns (making it very hard to read and edit) the form looks crappy since there are now p tags surrounding the input and button.

https://user-images.githubusercontent.com/1124594/26957265-e28714b0-4c91-11e7-9582-315dfbfbe3e3.png

<p>
Stay updated with our latest news and specials. We never sell your information and you can unsubscribe at any time.
</p>

<div class="custom-form-class">
	<form action="#" method="post" name="mc-embedded-subscribe-form">

		<label class="screen-reader-text" for="mce-EMAIL-b">Email </label>
		<input id="mce-EMAIL-b" class="required email" name="EMAIL" required="" type="email" value="" placeholder="Email Address*" />

		<input class="button" name="subscribe" type="submit" value="Go!" />

	</form>
</div>

#21 @carasmo
7 years ago

So, scanning through I wonder why it was wrong to use the text widget which indicates "arbitrary html" in its description. Many, many (nearly all) Genesis child themes don't come with a builder, so most of the time the home.php or the front-page.php is a widgetized page with areas in the content area for arbitrary html or some loop widget. It would be overkill to use anything else for arbitrary html but you want html with comments and returns so that users can read it easily and change a value or some text between the tags and use it.

#22 @synavista
7 years ago

What the previous Text Widget should or should not be used for is the topic for another debate (and has already been discussed in the #core Slack channel: https://wordpress.slack.com/archives/C02RQBWTW/p1496953943793348). At this point, it's been agreed upon that there are some (expected) conversion issues regarding certain HTML content. I think what's important is figuring out what the next steps are to ensure users are not losing content as a result of the upgrade to the new TinyMCE Text Widget, and it would appear the devs are assessing the options and working on possible solutions as we speak.

#23 @wolly
7 years ago

#40968 was marked as a duplicate.

This ticket was mentioned in Slack in #core by westonruter. View the logs.


7 years ago

#25 @westonruter
7 years ago

Also from @samuelsidler:

What about a mixed solution? Create the new HTML widget, as planned, but also add a notification of sorts, when a user edits an existing, pre-4.8 Text widget for the first time. Something like... "Hey, looks like you have a <script> tag; I've made this into an HTML widget for you." (Expand to other common HTML soup cases.)

Not ideal, but prevents data loss and lets us move forward with the Text widget.

To which I replied that it is similar to what is proposed in this ticket, but also:

But in that case, the suggestion is to disable the new experience altogether if it is an old Text widget that has the filter flag set to false, but the heuristics also then look at the content itself to see if it has script tag or other HTML soup that is liable to be lossy.

#26 follow-up: @vfontj
7 years ago

Christina asked that someone finish her plugin and submit it to the repository, which I've done. Until it is approved, you can download the classic text widget plugin here: https://victorfont.com/go/classic-text-widget/

You can also get the source code from Github here: https://victorfont.com/go/classic-text-widget-source/

#27 in reply to: ↑ 26 @carasmo
7 years ago

Replying to vfontj:

Christina asked that someone finish her plugin and submit it to the repository, which I've done. Until it is approved, you can download the classic text widget plugin here: https://victorfont.com/go/classic-text-widget/

You can also get the source code from Github here: https://victorfont.com/go/classic-text-widget-source/

Thank you Victor!

#28 @westonruter
7 years ago

#40980 was marked as a duplicate.

#29 @westonruter
7 years ago

From duplicate #40980:

Unlike editing pages, the new widget visual editor on 4.8 will break HTML formatting on new and existing text widgets when switching back and forth (even without saving).

Also of note (though this is probably a wpautop thing): on our installation, this also started adding <br> and <p> tags around comments and empty spaces where there were none before (we disable wpautop globally); we haven't had time to investigate so I can't be sure this visual editor is the cause, but it was worth noting.

#30 @westonruter
7 years ago

In 40893:

Widgets: Add widget dedicated for HTML Code, taking over this role of the Text widget.

Props westonruter, timmydcrawford.
See #40951, #35243.
Fixes #40907.

#31 follow-up: @And_or
7 years ago

Usually I am fast with updating websites, but this 'handy' Text Widget is a game-changer. Basically I feel punished for knowing and using html now.

I tested a simple thing like :
<span class="sectiondown"><a href="#front-page-3"><i class="fa fa-chevron-circle-down"></i></a></span>

It was invisible in the editor until I clicked save, then it appeared and had been wrapped in <code> tags thus changing the visual output. And next on opening the widget it was invisible again.
So I have convinced my clients to update their sites, and now I can not until I have manually checked and fixed a 100+ Genesis websites...which is going to take me days.
(Who is the genius that wanted this new widget?)

#32 in reply to: ↑ 31 @carasmo
7 years ago

Replying to And_or:

Usually I am fast with updating websites, but this 'handy' Text Widget is a game-changer. Basically I feel punished for knowing and using html now.

I tested a simple thing like :
<span class="sectiondown"><a href="#front-page-3"><i class="fa fa-chevron-circle-down"></i></a></span>

It was invisible in the editor until I clicked save, then it appeared and had been wrapped in <code> tags thus changing the visual output. And next on opening the widget it was invisible again.
So I have convinced my clients to update their sites, and now I can not until I have manually checked and fixed a 100+ Genesis websites...which is going to take me days.
(Who is the genius that wanted this new widget?)

Read this: https://christinacreativedesign.com/how-to-bulk-replace-the-4-8-wordpress-text-widget-with-the-classic-text-widget/

#33 @afercia
7 years ago

#40993 was marked as a duplicate.

#34 @westonruter
7 years ago

#41003 was marked as a duplicate.

#35 follow-up: @icodina
7 years ago

  • Resolution set to invalid
  • Status changed from new to closed

For those of us who use the text widget with HTML, JS or PHP code:
In order to copy and transfer the HTML, JS or PHP code to another widget or plugin, before editing, rename and replace the class php file of the widget (wp-includes/widgets/class-wp-widget-text.php) with the version prior to 4.8
This works correctly

Last edited 7 years ago by icodina (previous) (diff)

#36 in reply to: ↑ 35 @FolioVision
7 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to icodina:

For those of us who use the text widget with HTML, JS or PHP code:
In order to copy and transfer the HTML, JS or PHP code to another widget or plugin, before editing, rename and replace the class php file of the widget (wp-includes/widgets/class-wp-widget-text.php) with the version prior to 4.8
This works correctly

And why pray tell can we not do this automatically and without causing pain to millions of publishers and site administrators all over the world as suggested above by @carasmo and @And_or. I've read Carasamo's link to christinecreativedesign.com. Very painful. Fairly confusing. Generally unpleasant.

Breaking publishers' sites is incredibly arrogant. Why do WordPress Core and Jetpack think this is kind of behaviour (breaking what is not yours) is okay?

Let's just fix this automatically so people can get on with their lives and publishing, rather than fighting the tools.

Thanks, Alec

#37 follow-ups: @synavista
7 years ago

The more I think about (and deal with) this situation on the sites I manage, the more I'm convinced that it was (unfortunately) poor planning to take an existing widget that openly allowed "Arbitrary Text and HTML" (whether "proper" HTML or not) and then alter it's functionality such that people could no longer use it as it was previously available to be used.

The Text Widget is what should have been converted to the HTML Code widget, and something like a "Rich Text Editor" should have been added. A note to the user to "Check out the new Rich Text Editor" could be present on the original widget, notifying users to the newly available functionality. This way everyone who has existing content (no matter what) in a widget won't be affected, and people who wanted to use the new widget could do so on their own accord.

Additionally, based on the discussion in both #40907 and #core, it seemed like the decision to get the new Text Widget released was more important than, at a minimum, simultaneously releasing an HTML Code alternative and, at best, developing a solution that, while maybe not the absolute best user experience, at least didn't affect actual content and functionality and a root user level.

The option of A) replacing the new text widget with an unsupported "classic text widget" plugin, and modifying database tables to accommodate, is terribly inconvenient, and the option of B) modifying core files is absolutely not going to happen (every developer knows NOT to go editing core files).

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

Last edited 7 years ago by synavista (previous) (diff)

#38 in reply to: ↑ 37 @carasmo
7 years ago

Replying to synavista:

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

Yes!

#39 @icodina
7 years ago

@FolioVision, @synavista, I used text-widget to include JS, jQuery and PHP scripts and when editing with version 4.8 some parts of the code are lost.
Replacing wp-class-widget-text.php with the previous version before editing is not the solution, but allows us to retrieve our code.

#40 @anonymized_11892634
7 years ago

  • Severity changed from minor to normal

Every other CMS I can think of uses two distinct widgets/elements for HTML or WYSIWYG. For example Squarespace, Wix and Weebly all do it this way, as well as WordPress plugins such as Visual Composer and Beaver Builder etc.

The best approach, in my opinion, would be to follow this and create a dedicated "HTML" widget (which replaces the <4.8 "Text" widget. Then create a new widget called "Visual Edtior" which includes TinyMCE. When I first saw the announcement of this widget, this is what I assumed would be the case.

#41 in reply to: ↑ 37 @FolioVision
7 years ago

Replying to synavista:

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

Great idea.

Most sensible developers and agencies have not upgraded to 4.8.x yet after the debacle of the 4.7.0 and 4.7.1 hacks so doing the right thing now would both 1. mean most publishers don't have to deal with this self-inflicted wound 2. restore some confidence that the WordPress core team still has some interest in actually helping people publish rather than poisoning their lives by breaking their sites with every update.

@philclothier Your suggestion is also very clear and right on target.

Every other CMS I can think of uses two distinct widgets/elements for HTML or WYSIWYG. For example Squarespace, Wix and Weebly all do it this way, as well as WordPress plugins such as Visual Composer and Beaver Builder etc. The best approach, in my opinion, would be to follow this and create a dedicated "HTML" widget (which replaces the <4.8 "Text" widget. Then create a new widget called "Visual Edtior" which includes TinyMCE. When I first saw the announcement of this widget, this is what I assumed would be the case.

Makes a lot more sense to add a useful feature than to destroy an existing one and existing websites. If WordPress Core are hell bent on removing the text widget (why on earth do that - there is this insanity of wanting to bring WordPress to the lowest common denominator, ignoring the existence of Typepad, Squarespace, Wix and Weebly who cover this market far, far better), then they can deprecate the text widget over the course of 5 single point releases (i.e. not removing it until 5.4 or so with plenty of notice). By then someone could take the time to write a decent migration tool.

@westonruter @afercia @matt @markjaquith @helen

Breaking publishers' websites on updates is wrong, anti-productive and in the end only generates hostility to WordPress. It's an own goal.

#42 @majick
7 years ago

Can really only echo the above sentiments, and suggest the following sane steps for damage control:

  1. Revert the "new" Text Widget to the "classic" one from 3.7.1
  2. Add the 3.8 widget with the Visual Editor as a new widget. ("Visual Editor Widget"?)
  3. Do this by 3.8.1 at the latest to reduce the number of sites impacted.
  4. Ensure more discussion before releasing features that can strip formatting in the future.

#43 follow-up: @gitlost
7 years ago

+1 to points 1 to 3 above.

An alternative to the Classic Text Widget approach above is to disable the new functionality on the existing Text Widget by removing filters and javascript and overriding the widget registration, which seems to work fine - I've put this into a plugin which is available on https://github.com/gitlost/ye-olde-text-widget, the zip can be downloaded here.

(I don't know what sort of stuff I've kludged into text widgets over the years so installing this gives me peace of mind on upgrading to 4.8.)

Re point 4 I was following this development vaguely all along (the new widget is lovely) and still didn't click it could break my sites...

#44 @fullworks
7 years ago

As the 'add paragraphs' checkbox is supported backwardly compatible, I have published a plugin the the repo that add the checkbox back. This seems to sort most issues.

https://wordpress.org/plugins/add-paragraphs-option-to-text-widget/

#45 in reply to: ↑ 43 @And_or
7 years ago

Replying to gitlost:

+1 to points 1 to 3 above.

An alternative to the Classic Text Widget approach above is to disable the new functionality on the existing Text Widget by removing filters and javascript and overriding the widget registration, which seems to work fine - I've put this into a plugin which is available on https://github.com/gitlost/ye-olde-text-widget, the zip can be downloaded here.

(I don't know what sort of stuff I've kludged into text widgets over the years so installing this gives me peace of mind on upgrading to 4.8.)

Re point 4 I was following this development vaguely all along (the new widget is lovely) and still didn't click it could break my sites...

Thanks a lot @gitlost your plugin saves me a lot of hassle on some sites that had already updated to 4.8! (was so confident with WordPress I had set some sites to auto update... will be a while before that trust is back)
Can confirm it also works on MU

#46 @lukefiretoss
7 years ago

@gitlost

Just tested your plugin on a dev site, works great, thanks for the plugin on GitHub.

#47 @FolioVision
7 years ago

  • Severity changed from normal to major

Thanks for your efforts @gitlost. Unfortunately it doesn't really solve the issue and means that WordPress 4.8 updates can only really be done by experts.

@matt

It remains that breaking people's sites arbitrarily and more or less just for the heck of it is a completely unacceptable way to run WordPress updates.

I'm upgrading this bug to major as it's breaking people's sites and causing people to distrust WordPress updates as @And_or so eloquently noted above.

#48 follow-up: @gitlost
7 years ago

Thanks @And_or and @lukefiretoss - good to hear!

There's a 1.0.1 version available zip which makes sure that if after widget update the "Automatically add paragraphs" option is set then wpautop is applied, so it's more fully BC (cogged off @fullworks' plugin - thanks!).

@FolioVision - thanks - though re breaking sites it should I think be re-stated that (as mentioned above by various people) it's only a breaking change if the widget instance is actually updated in the admin.

#49 @fullworks
7 years ago

@gitlost thanks for the mention

#50 in reply to: ↑ 48 @And_or
7 years ago

Replying to gitlost:

Thanks @And_or and @lukefiretoss - good to hear!

There's a 1.0.1 version available zip which makes sure that if after widget update the "Automatically add paragraphs" option is set then wpautop is applied, so it's more fully BC (cogged off @fullworks' plugin - thanks!).

@FolioVision - thanks - though re breaking sites it should I think be re-stated that (as mentioned above by various people) it's only a breaking change if the widget instance is actually updated in the admin.

I am more with @FolioVision on this. Only breaking when updated is a misleading behavior, when the widget contains 'empty' html elements, the initial opening of the new Text Widget will not show anything, so the reaction is "What the heck" and you click the text-tab... and then still you see nothing. Until you click 'update' then you see the html content, which next time you look will in several cases be gone.
The Text Widget is to me 'The mother of all Widgets' - and you know what they call guys who mess with their mothers...

#51 @anonymized_11892634
7 years ago

FWIW, I agree with the bump up to major. The company I work for (a theme shop) has started to receive more and more support requests relating to this. We've had to adapt documentation and add extra screenshots to ensure people are changing the Text widget to "HTML" mode and that they strip out any extra line breaks.

This might seem like small steps to "tech savvy" people, but to the average WordPress user (90% of our clients) this appears to be causing aggravation (yet another reason to consider switching to a less scary CMS like Squarespace). We've also had feedback from people who are now avoiding updates because "it breaks things". We're always fighting an uphill battle with persuading clients to keep their themes/plugins up to date, so this issue really isn't helping :/

Last edited 7 years ago by anonymized_11892634 (previous) (diff)

#52 in reply to: ↑ 37 @gekkocorp
7 years ago

Totally agree. Several our text widgets with HTML have been destroyed.

Replying to synavista:

The more I think about (and deal with) this situation on the sites I manage, the more I'm convinced that it was (unfortunately) poor planning to take an existing widget that openly allowed "Arbitrary Text and HTML" (whether "proper" HTML or not) and then alter it's functionality such that people could no longer use it as it was previously available to be used.

The Text Widget is what should have been converted to the HTML Code widget, and something like a "Rich Text Editor" should have been added. A note to the user to "Check out the new Rich Text Editor" could be present on the original widget, notifying users to the newly available functionality. This way everyone who has existing content (no matter what) in a widget won't be affected, and people who wanted to use the new widget could do so on their own accord.

Additionally, based on the discussion in both #40907 and #core, it seemed like the decision to get the new Text Widget released was more important than, at a minimum, simultaneously releasing an HTML Code alternative and, at best, developing a solution that, while maybe not the absolute best user experience, at least didn't affect actual content and functionality and a root user level.

The option of A) replacing the new text widget with an unsupported "classic text widget" plugin, and modifying database tables to accommodate, is terribly inconvenient, and the option of B) modifying core files is absolutely not going to happen (every developer knows NOT to go editing core files).

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

#53 @fullworks
7 years ago

@gitlost taking a bit of your code I have extended my plugin now to start with no visual editor.
As it is silent if installed on 4.7 you can pre-install.
In theory then all will be good on upgrade.
If you want the visual editor later when happy - I have added a wp-config constant.

I'm just finishing testing and pushing to the repo, look out for version 1.3. https://wordpress.org/plugins/add-paragraphs-option-to-text-widget/

If any one has a 4.7 test site to try for upgrade I welcome further tests.

Last edited 7 years ago by fullworks (previous) (diff)

#54 follow-up: @icodina
7 years ago

@fullworks, I tested...
whitout the constant and with define('VISUAL_TEXT_WIDGET',false); allright, but with define('VISUAL_TEXT_WIDGET',true); does not work, the title and textarea do not appear, only the paragraph check

Last edited 7 years ago by icodina (previous) (diff)

#55 in reply to: ↑ 54 ; follow-up: @fullworks
7 years ago

Replying to icodina:

@fullworks, I tested...
whitout the constant and with define('VISUAL_TEXT_WIDGET',false); allright, but with define('VISUAL_TEXT_WIDGET',true); does not work, the title and textarea do not appear, only the paragraph check

Hi what version of PHP are you on?

#56 @icodina
7 years ago

5.6.17

...I changed that domain to PHP version 7 and it still does not work

Last edited 7 years ago by icodina (previous) (diff)

#57 in reply to: ↑ 55 @fullworks
7 years ago

The if statement is the same - so maybe you have Ye Olde plugin active too - that would kill the visual editor but leave the check box. Can you check what plugins you have activated?

Replying to fullworks:

Replying to icodina:

@fullworks, I tested...
whitout the constant and with define('VISUAL_TEXT_WIDGET',false); allright, but with define('VISUAL_TEXT_WIDGET',true); does not work, the title and textarea do not appear, only the paragraph check

Hi what version of PHP are you on?

#58 follow-up: @icodina
7 years ago

Active plugins in this site:

Add Paragraphs Option to Text Widget
Akismet Anti-Spam
Asesor de Cookies
Duplicate Post
Easy Smooth Scroll Links
Enable Media Replace
Enable Shortcode and PHP in Text widget
GT3 Photo & Video Gallery
Livemesh SiteOrigin Widgets
MainWP Child
Meta Slider
Page Builder by SiteOrigin
SiteOrigin Widgets Bundle
Ultimate Addons for SiteOrigin
Widgets for SiteOrigin
Wordfence Security

The only thing that could affect is "Enable Shortcode and PHP in Text widget", but I have already disabled it and neither

#59 in reply to: ↑ 58 @fullworks
7 years ago

We should take this offline, save cluttering an important track -> alan at fullworks.net

Replying to icodina:

Active plugins in this site:

The only thing that could affect is "Enable Shortcode and PHP in Text widget", but I have already disabled it and neither

#60 @FolioVision
7 years ago

There are three related open tickets about the issues which the forced upgrade of existing text widgets has caused:

I'm not expert enough in WordPress Trac to be able to etiquette to merge all three but perhaps someone else is.

Apparently the reason we are breaking millions of WordPress sites is fairly arbitrary decision by Matt made in Slack (hardly seem a very earnest way of running an open source project, taking breaking decisions in a real time chat environment with no opportunity for input by those not present at the specific meeting).

Matt wants to move forward with the upgraded Text widget as-is, with there not being a great enough concern for some users potentially having to adjust their usage to align with the new behavior (which aligns with longstanding post editor behavior).

Consensus among developers is synavista's suggestion:

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content).

Advocates of that point of view include many experienced developers and contributors such as sami.keijonen, Kopepash, carasmo, pipdig, piggybanktechnology, theMikeD, Cynderella, dfterry, skoen, j893, dwrippe, And_or, philclothier, majick, gitlost, gekkocorp.

The current fix strategy will not solve unnecessary breaking issues, as TheMikeD notes:

That's great, but it still means that every site that has ever used to code in the text widget now has to be manually updated. Backwards compatibility wise, this decision was not well thought out. I would like to see the rich text parts of the text widget reverted and a new rich text editor in trunk. In other words, don't replace the one that's already there, add a new one.

The update burden for those of us who actually manage WordPress websites in the real world (and don't have 500 developers on staff to do it for us) is substantial:

Usually I am fast with updating websites, but this 'handy' Text Widget is a game-changer. Basically I feel punished for knowing and using html now....So I have convinced my clients to update their sites, and now I can not until I have manually checked and fixed a 100+ Genesis websites...which is going to take me days.(Who is the genius that wanted this new widget?)

There's nothing wrong with making mistakes, Matt. There's something really wrong in not acknowledging one's mistakes and correcting them.

All we have to do here is make the HTML widget a separate widget and not auto-convert the existing widgets to an HTML widget. There's no problem even making the default widget going forward the HTML one. But force converting and breaking sites is not fair. I'm left pretty speechless by this deliberate sabotage of millions of sites.

Nonsense like this makes me very glad we created BusinessPress to block forced or inadvertent updates for WordPress until we're satisfied those updates no longer compromise security or functionality.

Millions of personal and business sites should not be a playground for beta software, whether that software is free or not. WordPress is not free to its users: they pay in spades for choosing WordPress in either money (management) or time (lots of their own free time). Honestly we should keep WordPress users' costs to a minimum and strive to keep the trust high.

#61 @programmin
7 years ago

To my knowledge TinyMCE has always done some odd stuff to html code (eg #41049 #20943 etc.), which is the reason many people plug code into the text widget in the first place! Seems MCE is taking out css class names, removing entire empty elements etc.

https://www.zigpress.com/2017/06/14/the-wordpress-4-8-text-widget-will-destroy-your-content/

This ticket was mentioned in Slack in #core by westonruter. View the logs.


7 years ago

#63 follow-up: @westonruter
7 years ago

  • Owner set to westonruter
  • Status changed from reopened to accepted

This was discussed in the core dev chat. See the conversation log. I started the conversation by pasting the following writeup:

To summarize the problem: the Text widget has been WordPress's de facto place to dump arbitrary HTML into sidebars. While arbitrary HTML can also be pasted into the post editor, it is there essentially managed by TinyMCE and will enforce wpautop while also stripping out empty elements and other markup that it determines is extraneous. When TinyMCE was introduced on the Text widget (#35243) it caused such custom-crafted HTML to get mutated by TinyMCE in undesirable ways.

By late in the 4.8 cycle the only known issue from testing was for Text widgets that had relied on wpautop being off, and for such instances the direction for users was to remove the extra paragraphs that would get added once the Text widget was modified. At the time, the only known issue was extra paragraphs being inserted, per comments on the Make/Core post: https://make.wordpress.org/core/2017/05/23/addition-of-tinymce-to-the-text-widget/

Important to note that Text widgets previously saved would retain the old behavior when rendering until the Text widget was updated with the new TinyMCE editor loaded. Since the presence of extra paragraphs was the only known issue, at the time late in the release cycle it was determined that reverting the Text widget (and renaming to HTML widget) and introducing a separate Rich Text widget was too drastic as a course of action. However, after the 4.8 release there appear to be a significant number of advanced users who are impacted by the change (see #40951), and the ultimate number of impacted users is not known now since, as noted above, the Text widget only changes its behavior once it is modified.

In trunk there is presently an “HTML Code” widget for storing arbitrary HTML code that is not managed by TinyMCE. See #40907.

One proposed solution was to convert all existing Text widgets over to HTML Code widgets as part of an upgrade routing. This will not have the desired effect for some advanced users because it will change the widget's ID that contains the content, and this will break CSS selectors that target the widget. It can also have negative impacts for plugins that have existing references to Text widget IDs for placement in contexts outside normal dynamic_sidebar() calls. Even for non-advanced users, migrating existing Text widgets over to HTML Code widgets is not ideal because then novice users, who may not know HTML to begin with, will have difficulty locating the widget that has the content they need to modify. Seeing the word “HTML” may also confuse and intimidate them.

Another solution proposed was to revert the Text widget back to its original state (maybe renaming to HTML Code widget) and introduce a new Rich Text widget. However, a key reason for letting the existing Text widget remain upgraded with TinyMCE is that novice users who previously have just put simple text into the widget should by default get the new visual editing experience. Novice users shouldn't have to be bothered with copy/pasting the contents of their widget into a separate widget in order to start doing visual editing, in the same way that they don't have to switch between post editing experiences.

However, I think the best suggestion that has been proposed is this: when loading the form for a Text widget, just as on the frontend it checks to see if it was previously saved from an older version of WordPress before TinyMCE was added to the Text widget. If it is such a pre-existing Text widget instance, then use heuristics to detect if TinyMCE would negatively impact the contents of the widget, including the auto-p checkbox being unchecked, whether there are empty tags, and whether there are span, div, script, or style tags. When the Text widget is in this legacy mode, it can have a notice that informs users of the new HTML Code widget and that it should be used going forward. Likewise, in the new mode when TinyMCE is present, when the Text (HTML) tab is selected, there can be a note (perhaps an admin pointer) that encourages users to use the HTML Code widget instead. By implementing this, novice users with basic content in their widgets win, and advanced users with custom HTML content in their widgets will cease from being negatively impacted.

Based on the core dev chat discussion, I will be moving forward with implementing the proposed suggestion in this last paragraph, to introduce a legacy mode to the Text widget and use it when a widget instance contains non-wpautop content or advanced HTML. See the Slack log for the full conversation.

#64 in reply to: ↑ 63 @synavista
7 years ago

Replying to westonruter:

By implementing this, novice users with basic content in their widgets win, and advanced users with custom HTML content in their widgets will cease from being negatively impacted.

Thanks, @westonruter. This sounds like a good middle ground for both novice and advanced users. Any hope on this functionality being included in the 4.8.1 release?

#65 @anonymized_11892634
7 years ago

Thanks for the update @westonruter. FWIW I think that plan makes sense and I'm now looking forward to 4.8.1

#66 @melchoyce
7 years ago

#41120 was marked as a duplicate.

#67 @FolioVision
7 years ago

Thanks Weston for the update on current status and the detailed summary of the offsite discussion. Quite honestly Trac is where this kind of conversation belongs if core wants to be inclusive and not exclusive. You proposed:

I think the best suggestion that has been proposed is this: when loading the form for a Text widget, just as on the frontend it checks to see if it was previously saved from an older version of WordPress before TinyMCE was added to the Text widget. If it is such a pre-existing Text widget instance, then use heuristics to detect if TinyMCE would negatively impact the contents of the widget, including the auto-p checkbox being unchecked, whether there are empty tags, and whether there are span, div, script, or style tags. When the Text widget is in this legacy mode, it can have a notice that informs users of the new HTML Code widget and that it should be used going forward.

To my mind this seems far too complex and prone to false negatives. It's more a face saver for the original very wrong policy. What would be reliable and simple would be: To detect the kind of widget. If it's an older text widget just leave it as a text widget. Nothing breaks on 4.8.1 upgrades. Confidence restored in WordPress updates.

Going forward novice users can use the HTML widget (I hope that prompt not to use the very robust and useful text widget won't be too annoying). In this case WordPress core won't be breaking everyone's site because one person arbitrarily decided to kill WordPress's core widget in a futile hope of appealing to SquareSpace users and pushing market share another 0.5 per cent higher instead of actually improving WordPress and making the lives of existing users better.

PS. Improving the lives of existing users would be the better way to increase marketshare. Persuade people to bring their friends instead of sending them running from the hills due to the high maintenance costs and admin time required just to survive such updates.

#68 follow-up: @westonruter
7 years ago

@synavista yes, this will be part of 4.8.1

@FolioVision:

Thanks Weston for the update on current status and the detailed summary of the offsite discussion.

Actually, it wasn't offsite. It was in Slack, in an open meeting. I literally copy/pasted here what I pasted into Slack. I was going to include it here regardless.

It's more a face saver for the original very wrong policy. What would be reliable and simple would be: To detect the kind of widget. If it's an older text widget just leave it as a text widget. Nothing breaks on 4.8.1 upgrades.

It's not a face saver solution. I honestly think it's the best solution and a legacy mode should have been done this way from the start. It has in mind both the best interests of the majority of users who are novices with basic HTML (or zero HTML with just plain text), and also the interests of advanced users with custom HTML. The Text widget should have had TinyMCE when it was first introduced in WordPress 2.2.

It may be that the compatibility detection is not feasible to detect whether a Text widget can be upgraded to use the TinyMCE by inspecting the widget instance, and in such case the preexisting widgets could always be in legacy mode. I think the detection can be done, however. For example, if a widget's text has no line breaks or if it has the autop checkbox enabled, and if there are only strong, em, a[href], img, ul, ol, li tags in the content, without any other attributes added to the elements, then there would not be anything in the widget that would be incompatible with TinyMCE.

Naturally, we'll be looking to all your guys's help to test and confirm any assumptions that the widget takes in its legacy mode. Thanks all.

#69 in reply to: ↑ 68 ; follow-up: @anonymized_11892634
7 years ago

Replying to westonruter:

It's not a face saver solution. I honestly think it's the best solution and a legacy mode should have been done this way from the start. It has in mind both the best interests of the majority of users who are novices with basic HTML (or zero HTML with just plain text), and also the interests of advanced users with custom HTML. The Text widget should have had TinyMCE when it was first introduced in WordPress 2.2.

With respect, this is the most "face saving" solution I've seen in a long time. Whilst I appreciate that this issue will be resolved in 4.8.1, I'm concerned that you still think that this was a good idea from the start. I can't think of any reason why this would be chosen over creating a new, separate TinyMCE widget. Then you get the best of both worlds rather than a fudged mix of the two. People who have used plain text in the old widget can happily upgrade to the new TinyMCE widget if they want to, and think "ooh shiny new widgets. This is great!". People who have used HTML can relax and leave it as is, without having to worry that the next update will break anything. Is there something else I'm missing here?

For example, if the approach is to add new functionality to current widgets, why bother creating the new Image, Audio and Video widgets? Those features could have been added to the new Text widget as buttons.

#70 @majick
7 years ago

This is a bit of a complex one to approach since we now have to deal with:

  1. As yet unchanged Text Widget content (pre-4.8)
  2. Possibly modified Text Widget content (post-4.8)
  3. New User Content (post-4.8)

Since rolling back is unlikely to be backwards-compatible with going forward (lolz) I'd like to propose an alternative solution...

  1. Make TinyMCE Disabled by Default in the current (new) "Text Widget"
  2. Add a checkbox to the current Text Widget options to Enable TinyMCE for that Widget instance.
  3. Add a message "Please recheck your formatting if switching to Visual Text" when box is checked.
  4. Add a "new" Widget (class extend?) called "Visual Text" with TinyMCE Enabled by Default (thus with the checkbox already checked also.)
  5. Let users choose whether to use the old or new Widget when adding it, the only difference really being that TinyMCE is enabled or disabled by default depending on which type is added, the "Text" or "Visual Text" Widget (and also keep the class names etc. the same to prevent CSS change of targetting problems as already noted.)

Best of both worlds. Existing Text Widgets content not affected, and minimal breakage going forward.

Last edited 7 years ago by majick (previous) (diff)

#71 in reply to: ↑ 69 ; follow-up: @westonruter
7 years ago

Replying to philclothier:

I can't think of any reason why this would be chosen over creating a new, separate TinyMCE widget. Then you get the best of both worlds rather than a fudged mix of the two. People who have used plain text in the old widget can happily upgrade to the new TinyMCE widget if they want to, and think "ooh shiny new widgets. This is great!". People who have used HTML can relax and leave it as is, without having to worry that the next update will break anything. Is there something else I'm missing here?

As I tried to convey in my comment above, the reason for upgrading the Text widget as opposed to introducing a new widget type is for the sake of users who aren't doing anything advanced in their widgets. They get the new experience by default and don't have to do any migration. The Text widget should have supported TinyMCE from the start when the widget was introduced in WordPress 2.2, so this is essentially fixing a long-standing deficiency.

For example, if the approach is to add new functionality to current widgets, why bother creating the new Image, Audio and Video widgets? Those features could have been added to the new Text widget as buttons.

Actually there is a ticket for facilitating the addition of media to the TinyMCE-enhanced Text widget as well. See #40854. And there were actually discussions about whether to just re-use the Text widget and just add media to it. Instead we decided on separate media widgets. The new media widgets for images, audio, and video were created to give tailored experiences and shortcuts for adding specific media types. Having separate widgets for each media type also improves discoverability.

This ticket was mentioned in Slack in #forums by t-p. View the logs.


7 years ago

#73 @melchoyce
7 years ago

#41129 was marked as a duplicate.

#74 in reply to: ↑ 71 @anonymized_11892634
7 years ago

Replying to westonruter:

The Text widget should have supported TinyMCE from the start when the widget was introduced in WordPress 2.2, so this is essentially fixing a long-standing deficiency.

In my mind, it would have made more sense to rename the Text widget "HTML" then create a new Visual widget called "Text" which faux-replaced the old version. Then you get 100% backwards compatibility and the new features. Everyone's a winner.

Anyone which was desperate for this kind of functionality has long been using https://wordpress.org/plugins/black-studio-tinymce-widget/ anyway. So I don't see why "They get the new experience by default and don't have to do any migration" was a high priority. People were using the Text widget for years - they know what it can and can't do.

Anyway, I'll stop being a negative-nancy. What's done is done. I just wish you'd admit it was a bad idea like everyone else is thinking :D

@westonruter
7 years ago

Mock of Text widget in legacy mode

#75 @melchoyce
7 years ago

Looking good!

We'll want to add the .notice-alt class to this since the notice is on a white background.

I chatted a bit with @reportermike and @kjellr about the copy and we settled on:

This widget contains code that may be better used in the HTML Code widget. We recommend you use the HTML Code widget for this content.

We dropped the bit about the visual editor, because it might confuse more than help, especially if you aren't aware there is a visual editor now.

#76 follow-up: @fullworks
7 years ago

Sorry if this is a bit obvious and noob question to you all. But where can I get the pre-release code of 4.8.1 text widget, as I will need to work out how to handle the revisions in my plugin that reverts the text widget to pre 4.8 (Add Paragraphs Option to Text Widget). Whilst not massively installed, it has a growing install base, so I need to be clear so my plugin doesn't break sites in 4.8.1

#77 in reply to: ↑ 76 @fullworks
7 years ago

Don't worry - found it - at least where it should be pre release
Replying to fullworks:

Sorry if this is a bit obvious and noob question to you all. But where can I get the pre-release code of 4.8.1 text widget, as I will need to work out how to handle the revisions in my plugin that reverts the text widget to pre 4.8 (Add Paragraphs Option to Text Widget). Whilst not massively installed, it has a growing install base, so I need to be clear so my plugin doesn't break sites in 4.8.1

Last edited 7 years ago by fullworks (previous) (diff)

#78 @anonymized_11892634
7 years ago

Looks like a good solution so far. However I'd like to suggest the message be reworded to:

This widget includes content which may work better in the 'HTML Code' widget. We recommend that you use the 'HTML Code' widget for this type of content.

Thoughts?

Last edited 7 years ago by anonymized_11892634 (previous) (diff)

@westonruter
7 years ago

Second screenshot of Text widget in legacy mode

#79 @westonruter
7 years ago

  • Keywords needs-testing has-unit-tests added

OK, this is ready for testing. Please see 40951.0.diff which is also available in a PR on GitHub for checkout and review.

The patch introduces a legacy mode for the Text widget. The legacy mode hinges on the new \WP_Widget_Text::is_legacy_instance() method. It has an explicit whitelist of conditions that will result in a Text widget being initialized in legacy mode. Once a Text widget is determined to need legacy mode, after the widget is henceforth modified the legacy mode will persist in perpetuity for the instance. The whitelist of conditions for a Text widget being in legacy mode are:

  • It was created prior to the TinyMCE-enhanced widget in 4.8. All widgets created in 4.8 and beyond will never be in legacy mode but will always have TinyMCE.
  • The auto-paragraph checkbox was unchecked and there are line breaks in the text.
  • There are empty elements that are not always empty, like img or hr.
  • The text contains any HTML element or attribute other than: strong, em, b, i, u, s, ul, ol, li, hr, abbr, acronym, code, dfn, a[href], img[src][alt].

Question: Should style be whitelisted for all of these elements as well?

I added unit tests that include all of the examples shared in this ticket. See text-widget-legacy-mode-2.png for screenshot of what the legacy mode looks like.

@melchoyce For the non-legacy widget, should there be a notice displayed when selecting the Text (HTML) tab of the TinyMCE editor that they should also consider the HTML Code (Custom HTML) widget instead?

Please test.

#80 @westonruter
7 years ago

40951.1.diff adjust the notice text to be:

This widget contains code that may work better in the “HTML Code” widget. How about trying that widget instead?

#81 @melchoyce
7 years ago

@melchoyce For the non-legacy widget, should there be a notice displayed when selecting the Text (HTML) tab of the TinyMCE editor that they should also consider the HTML Code (Custom HTML) widget instead?

Good question — maybe yes? Here's a super rough idea:

Hey, did you hear we have a "Custom HTML" widget now? Check it out to add some custom code to your site!

Maybe as a dismissible notice, that remembers your preference if you close it.

#82 follow-up: @gitlost
7 years ago

I think the message here in legacy mode should be similar (and dismissible). It sounds a bit rich at the moment to be in effect saying "Oo, you shouldn't be doing it this way, you should be doing it that way" when that way didn't previously exist.

#83 in reply to: ↑ 82 @westonruter
7 years ago

Replying to gitlost:

I think the message here in legacy mode should be similar (and dismissible). It sounds a bit rich at the moment to be in effect saying "Oo, you shouldn't be doing it this way, you should be doing it that way" when that way didn't previously exist.

@melchoyce With that in mind, what about including the word “new” before Custom HTML widget, something like:

This widget contains code that may work better in the new “Custom HTML” widget. How about trying that widget instead?

Last edited 7 years ago by westonruter (previous) (diff)

#84 @FolioVision
7 years ago

Thanks for your hard work on this Weston and your detailed notes.

I prefer the second wording. Could we make it a bit more to the point?

This widget contains code that may work better in the new “Custom HTML” widget. Convert now.

A single click would convert the widget to the Custom HTML widget.

#85 @westonruter
7 years ago

@FolioVision Being able to click convert the widget type is a nice idea, but given the state of modeling widgets in JS (or rather lack of modeling) it is a very challenging thing to do presently (in contrast to blocks in Gutenberg which have built-in support for transformations).

@westonruter
7 years ago

Admin pointer shown upon clicking Text tab in Customizer

@westonruter
7 years ago

Admin pointer shown upon clicking Text tab in admin screen

#87 follow-up: @gitlost
7 years ago

In the old admin > Appearance > Widgets interface, "wp-admin/js/common.js:391" is moving the notice to the top of the screen.

#88 in reply to: ↑ 86 @melchoyce
7 years ago

Replying to westonruter:

@melchoyce With that in mind, what about including the word “new” before Custom HTML widget, something like:

This widget contains code that may work better in the new “Custom HTML” widget. How about trying that widget instead?

👍

Replying to westonruter:

@melchoyce Thoughts on admin-pointer-widgets-screen.png and admin-pointer-customizer.png?

I'll want to run the copy by some more folks, but I think that works in general 👍 Seems like a good use of pointers here.

#89 in reply to: ↑ 87 @westonruter
7 years ago

  • Keywords has-patch added; needs-patch removed

Replying to gitlost:

In the old admin > Appearance > Widgets interface, "wp-admin/js/common.js:391" is moving the notice to the top of the screen.

Thanks for reporting that. I noticed it too. Fixed in https://github.com/xwp/wordpress-develop/pull/235/commits/1815910

All: Pointer functionality can be tested in 40951.2.diff.

Note that after you dismiss the admin pointer, you can clear it out to test again via WP-CLI: wp user meta set $WP_USER_ID dismissed_wp_pointers ''

Should the pointer show more than one time ever for a user? Should it instead show once per browser session? (Using sessionStorage.)

I'm not very happy with how it is hacking the z-index to get the pointer to appear in the Customizer at the right elevation.

Again, code is available for review in a GitHub PR: https://github.com/xwp/wordpress-develop/pull/235/files

@westonruter
7 years ago

Refresh patch

#90 follow-ups: @WPwebbouw
7 years ago

One issue may have been overlooked in this discussion. If the text widget is being replaced by a new HTML widget, then also the filter

widget_text

will be renamed. This means that any code in a theme or widget that uses this filter must be changed to use the new filter name. Which means this solution would again require admins to manually check all sites where text widgets have been applied to see if this filter has been used.

In my opinion the only sane decision to be made is to revert the new text widget introduced in 4.8 back to the original text widget. Preferably in the 4.8.1 update. After that, introduce whatever wysiwyg text widget you want, but don't ever again do it in a way that destroys existing sites, kills existing functionality and forces admins to fix a problen they didn't create.

#91 in reply to: ↑ 90 @emmtre
7 years ago

Replying to WPwebbouw:

In my opinion the only sane decision to be made is to revert the new text widget introduced in 4.8 back to the original text widget. Preferably in the 4.8.1 update. After that, introduce whatever wysiwyg text widget you want, but don't ever again do it in a way that destroys existing sites, kills existing functionality and forces admins to fix a problen they didn't create.

I couldn't agree more. This proposed face saver solution is just crazy. Revert back and do it right. This is already a mess with all clients. Sorry but this is not a viable solution.

#92 follow-ups: @gitlost
7 years ago

In the refresh patch "40951.3.diff" the legacy bail out got moved to after the TextWidgetControl addition in component.handleWidgetAdded() so you get a blank textarea showing...

@WPwebbouw

If the text widget is being replaced by a new HTML widget

It's not being replaced but being made legacy compatible, with the suggestion to use the new HTML widget.

@emmtre

Sorry but this is not a viable solution.

Have you tried it? In my testing it works really well ("40951.2.diff" that is!).

#93 in reply to: ↑ 37 ; follow-up: @jdcohan
7 years ago

+100 for this.
Replying to synavista:

The more I think about (and deal with) this situation on the sites I manage, the more I'm convinced that it was (unfortunately) poor planning to take an existing widget that openly allowed "Arbitrary Text and HTML" (whether "proper" HTML or not) and then alter it's functionality such that people could no longer use it as it was previously available to be used.

The Text Widget is what should have been converted to the HTML Code widget, and something like a "Rich Text Editor" should have been added. A note to the user to "Check out the new Rich Text Editor" could be present on the original widget, notifying users to the newly available functionality. This way everyone who has existing content (no matter what) in a widget won't be affected, and people who wanted to use the new widget could do so on their own accord.

Additionally, based on the discussion in both #40907 and #core, it seemed like the decision to get the new Text Widget released was more important than, at a minimum, simultaneously releasing an HTML Code alternative and, at best, developing a solution that, while maybe not the absolute best user experience, at least didn't affect actual content and functionality and a root user level.

The option of A) replacing the new text widget with an unsupported "classic text widget" plugin, and modifying database tables to accommodate, is terribly inconvenient, and the option of B) modifying core files is absolutely not going to happen (every developer knows NOT to go editing core files).

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

#94 in reply to: ↑ 93 @fullworks
7 years ago

Well that is not going to happen - they are going to make it backwardly compatible. Its already in test,

Anyway that analysis referenced below missed out option C) which is not to modify core files - but to use the hooks & filters available in a plugin - which is what I did here https://wordpress.org/plugins/add-paragraphs-option-to-text-widget/ if you are desperate to update to 4.8 - although - subject to testing the value of my plugin will nearly completely disappear in 4.8.1

Replying to jdcohan:

+100 for this.
Replying to synavista:

The more I think about (and deal with) this situation on the sites I manage, the more I'm convinced that it was (unfortunately) poor planning to take an existing widget that openly allowed "Arbitrary Text and HTML" (whether "proper" HTML or not) and then alter it's functionality such that people could no longer use it as it was previously available to be used.

The Text Widget is what should have been converted to the HTML Code widget, and something like a "Rich Text Editor" should have been added. A note to the user to "Check out the new Rich Text Editor" could be present on the original widget, notifying users to the newly available functionality. This way everyone who has existing content (no matter what) in a widget won't be affected, and people who wanted to use the new widget could do so on their own accord.

Additionally, based on the discussion in both #40907 and #core, it seemed like the decision to get the new Text Widget released was more important than, at a minimum, simultaneously releasing an HTML Code alternative and, at best, developing a solution that, while maybe not the absolute best user experience, at least didn't affect actual content and functionality and a root user level.

The option of A) replacing the new text widget with an unsupported "classic text widget" plugin, and modifying database tables to accommodate, is terribly inconvenient, and the option of B) modifying core files is absolutely not going to happen (every developer knows NOT to go editing core files).

The best course of action at this point, it would seem, would be to admit the error, convert the text editor back to the way it was, and simultaneously release the updated HTML Code widget along with a new Rich Text Editor. If that is completely impossible, at least figure out a way for the TinyMCE Text Editor to both default to the text editor AND remember the selected editor type (which I would hope would stop the system from stripping out existing content). That makes it a pain in the butt for developers to go in and convert to the new HTML Code widget, but at least it makes it possible.

Last edited 7 years ago by fullworks (previous) (diff)

#95 in reply to: ↑ 92 @emmtre
7 years ago

Replying to gitlost:

@emmtre

Sorry but this is not a viable solution.

Have you tried it? In my testing it works really well ("40951.2.diff" that is!).

What works really well? That I have to convert hundreds of widgets on client websites to the new html widget so it won't break again in another update. And fix the styling for every single separat widget with forms, opt-ins, call for actions, footers, etc due to new class names and widget instances. Sorry if I'm negative but this kind of update that break websites is not ok in any way.

Last edited 7 years ago by emmtre (previous) (diff)

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


7 years ago

#97 in reply to: ↑ 92 ; follow-up: @westonruter
7 years ago

Replying to gitlost:

In the refresh patch "40951.3.diff" the legacy bail out got moved to after the TextWidgetControl addition in component.handleWidgetAdded() so you get a blank textarea showing...

Thanks very much for catching that! My merge conflict resolution put the condition in the wrong place. @obenland would you fix the latest patch to correct that? I'm on mobile this week.

#98 in reply to: ↑ 97 @westonruter
7 years ago

@gitlost actually, I'm not seeing what you are reporting. I see the legacy check being done before the widget control is instantiated. See https://github.com/xwp/wordpress-develop/pull/235/files#diff-a413842f2fc6c3fa90f8ea30d0f27b59R257

Could it be that your trunk wasn't fully up to date when you applied the patch? I had refreshed the patch due to a merge conflict for another commit I made immediately prior (see r40941).

#99 follow-up: @gitlost
7 years ago

@westonruter apologies my mistake, I applied it directly to a 4.8 site and the fuzzy matching put it in the wrong place.

@emmtre

What works really well?

The proposed solution works really well.

That I have to convert hundreds of widgets ...

You won't have to do that.

Last edited 7 years ago by gitlost (previous) (diff)

#100 in reply to: ↑ 99 @emmtre
7 years ago

@emmtre

What works really well?

The proposed solution works really well.

That I have to convert hundreds of widgets ...

You won't have to do that.

Well, ignorance is bliss...

#101 follow-ups: @obenland
7 years ago

For the admin pointers I noticed:

  • There's no way for users to take action. The pointer is meant to have them use the HTML widget, but it's unclear where to go next after reading the pointer. /cc @melchoyce
  • On widgets.php the pointer's high z-index doesn't allow it to slide under the admin toolbar
  • In the customizer the pointer's position is fixed and doesn't scroll with the widget.
  • If we could hide the pointer when switching back to visual mode, that would be great.
  • Pointers don't seem to use the WP_Internal_Pointers class, so they can't be disabled and new users still see them.

I think generally the introduction of a new pointer should be discussed in a dev chat.

In widgets.php, the new HTML widget control would probably benefit from being allowed to overflow the sidebar, similar to the text widget.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 years ago

#103 follow-up: @westonruter
7 years ago

One other thing, which was brought up by @pipdig in #40960: users are accustomed to pasting HTML into the Text widget, and they aren't used to first having to click the Text tab. It would probably be a good idea to show the pointer as soon as someone pastes what looks to be HTML code into TinyMCE. This would be useful perhaps generally for the WP editor, eh @azaozz?

#104 @westonruter
7 years ago

In 41000:

Widgets: Let Custom HTML widget be "wide" as the Text widget is.

Also update WP_Customize_Widgets::$core_widget_id_bases with the new list of core widgets.

Props obenland, westonruter.
See #40907, #40951.

#105 in reply to: ↑ 90 @westonruter
7 years ago

Replying to WPwebbouw:

One issue may have been overlooked in this discussion. If the text widget is being replaced by a new HTML widget, then also the filter widget_text will be renamed. This means that any code in a theme or widget that uses this filter must be changed to use the new filter name. Which means this solution would again require admins to manually check all sites where text widgets have been applied to see if this filter has been used.

The Text widget is not being replaced by the Custom HTML widget; it is being added alongside. For the Text widget, the widget_text filter remains on the Text widget. If the legacy mode is enabled, then this is the only filter that applies. If not in legacy mode, then that filter applies before the new widget_text_content filter. See https://github.com/WordPress/wordpress-develop/blob/4.8.0/src/wp-includes/widgets/class-wp-widget-text.php#L72-L98

If there are filters you want to apply to the Custom HTML widget, then any such functions should also be added to the widget_custom_html_content filter as well. If a case can be made, the widget_text filter could potentially be applied in the Custom HTML widget as well. For that, comment on #40907 with specific examples.

#106 @westonruter
7 years ago

In 40951.4.diff (diff) the legacy mode detection is expanded to also include whether or not shortcodes are present in the text, since if shortcodes get applied at widget_text then any HTML added could be corrupted by wpautop which then runs later at widget_text_content. Plugins that add shortcode support for the Text widget should use the widget_text_content filter at priority 11, in the same way that core applies shortcodes on the the_content filter at priority 11, whereas wpautop runs at priority 10.

#107 @westonruter
7 years ago

And 40951.5.diff (commit) is the complimentary change to the previous in that it temporarily upgrades shortcode handling from applying at widget_text to instead apply at widget_text_content after wpautop has already run, so that any HTML output by the shortcode won't be corrupted.

#108 follow-up: @azaozz
7 years ago

Related: #29171. Would appreciate some testing. Also, are there other tags that make sense to be used empty and are being stripped? I know <i> is not stripped any more (when it has attributes), are there others we need to add?

#109 in reply to: ↑ 103 ; follow-up: @azaozz
7 years ago

Replying to westonruter:

...users are accustomed to pasting HTML into the Text widget, and they aren't used to first having to click the Text tab.

That may be, but how often are the users editing their text widgets and how often are they editing or writing posts? IMHO the users are accustomed to use the editor much more than the text widget. It will be easy/convenient for them to (re)use that experience. No need of so much "hand holding" :)

#110 in reply to: ↑ 108 @ElectricFeet
7 years ago

Replying to azaozz:

Related: #29171. Would appreciate some testing. Also, are there other tags that make sense to be used empty and are being stripped? I know <i> is not stripped any more (when it has attributes), are there others we need to add?

I'm not sure if I'm answering the question you're asking, but I had problems with the following being deleted:

<div itemscope itemtype="http://schema.org/LocalBusiness">
<p itemprop="address" itemscope itemtype="http://schema.org/PostalAddress"><span itemprop="streetAddress">blah blah</span>Etc Etc</p>
<p>Tel:<span itemprop="telephone">12345678</span></p>
</div>

which I reported here: https://wordpress.org/support/topic/4-8-bug-in-widgets-with-tinymce/.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 years ago

#112 follow-up: @westonruter
7 years ago

Once the patch is committed, we're planning to create a 4.8.1-alpha release build that includes this legacy Text widget mode as well as the Custom HTML widget. Having a full release build will make it easier for testers to test. Along with that release build, testers need steps to follow to confirm it is working properly.

My quick take on testing steps would be to:

  1. Open a Text widget that contains custom HTML in WP admin and/or Customizer.
  2. Confirm that legacy mode is entered by the presence of the blue notice under the title, and that it is a simple textarea.
  3. Confirm that changes to the custom HTML in the widget get saved.
  4. Open Text widgets that are simple, without custom HTML, and that they get opened with TinyMCE enabled for visual editing.
  5. Try adding a new Custom HTML widget and add custom HTML there that users would formerly try adding to the Text widget. Confirm that the entered HTML appears as expected on the site, including Customizer preview.

What else should be included in the user testing steps?

We'll be looking to everyone watching this ticket in particular to assist with testing. 🙇

#113 in reply to: ↑ 112 @ElectricFeet
7 years ago

Replying to westonruter:

My quick take on testing steps would be to:
1 ...

I would add that someone needs to update the 4.8 known issues post asap. I really wish I hadn't upgraded my sites ... and I wouldn't have done if this bug had been in the issues list.

Great work going on here! Looks like fixing the bug is going to give us a nice new feature :)

This ticket was mentioned in Slack in #forums by clorith. View the logs.


7 years ago

#115 in reply to: ↑ 109 @Ipstenu
7 years ago

Replying to azaozz:

Replying to westonruter:

...users are accustomed to pasting HTML into the Text widget, and they aren't used to first having to click the Text tab.

That may be, but how often are the users editing their text widgets and how often are they editing or writing posts? IMHO the users are accustomed to use the editor much more than the text widget. It will be easy/convenient for them to (re)use that experience. No need of so much "hand holding" :)

Here's how it happens: I am used to writing posts and I know I can't use icons. I haven't update my widgets in forever, though, and I go to fix a URL or change one in a custom text widget and now my site looks like crap and all I did was save the widget! What happened?

And yes, that happened to me who knows better. I got it, right away, that the world had changed and why, but the same cannot be said of people who use the features even less often. It's a problem of rarely-executed code. People are less likely to recognize the reasons something broke/change if they don't have to dig into it very often. Based on the support forums, I think it's not huge, but definitely bigger than anticipated, so a bit of handholding is needed.

I have some edge cases to happily test when this hits trunk!

#116 in reply to: ↑ 101 @melchoyce
7 years ago

Replying to obenland:

There's no way for users to take action. The pointer is meant to have them use the HTML widget, but it's unclear where to go next after reading the pointer. /cc @melchoyce

What do you think about changing it to:

Hey, did you hear we have a "Custom HTML" widget now? You can find it by pressing the "Add a Widget" button and searching for "HTML." Check it out to add some custom code to your site!

Do you think that makes it actionable enough?

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


7 years ago

#118 in reply to: ↑ 101 @westonruter
7 years ago

Replying to obenland:

For the admin pointers I noticed:

  • There's no way for users to take action. The pointer is meant to have them use the HTML widget, but it's unclear where to go next after reading the pointer. /cc @melchoyce
  • If we could hide the pointer when switching back to visual mode, that would be great.

These issues are addressed in 40951.6.diff.

  • Pointers don't seem to use the WP_Internal_Pointers class, so they can't be disabled and new users still see them.

Can you elaborate? The pointers are disabled once the user clicks dismiss. Also, I think that it is good to show for new users, as for many people a new user just means a new install, and the users have worked with the Text widget before, or they are actioning on tutorials that advise to use the Text widget.

#119 @westonruter
7 years ago

40951.7.diff attempts to hide the pointer when the widget is collapsed. It does improve the positioning o the pointer to show on the right side of the widget (over the preview in the Customizer) but it does not yet handle keeping the pointer in place when scrolling the widget in the panel.

Honestly, because there are so many hacks required to get an admin pointer to work, I think it would be better just the notice inside the widget itself, so something like text-widget-legacy-mode-2.png, but maybe it should appear below the editor so that it doesn't annoy people when they try clicking between the tabs.

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


7 years ago

@westonruter
7 years ago

Mock of how dismissible notice could be used instead of admin pointer

#121 @Ov3rfly
7 years ago

We use schema.org structured data on many customer websites in a text widget which is completely stripped by current 4.8 behaviour, so we can not update for now. Example:

<p itemscope itemtype="http://schema.org/Organization"><strong itemprop="name">ACME</strong><br /><span itemprop="address" itemscope itemtype="http://schema.org/PostalAddress"><span itemprop="streetAddress">Hollywood Boulevard</span><br /><span itemprop="postalCode">12345</span> <span itemprop="addressLocality">Sin City</span><br /></span>Tel.: <span itemprop="telephone" content="+15552345567">555-2345567</span><br /><a itemprop="email" content="info@example.com" href="mailto:info@example.com?subject=FUUUU">info@example.com</a><meta itemprop="url" content="http://www.example.com/" /></p>

Please make sure these schema.org attributes and also <meta> tags are not stripped in future version, thanks for considering.

#122 @westonruter
7 years ago

@Ov3rfly yes, as you can see from microdata-retained-in-legacy-mode.png, if you have any pre-existing Text widget with any markup that is not in the “basic”, which TinyMCE is normally dealing with, then the widget will be presented in legacy mode. But going forward, use of the new Custom HTML widget will be the way to go for such markup.

#123 follow-up: @Ov3rfly
7 years ago

@westonruter On customer website after update to 4.8 the pre-existing above quoted code was stripped completely without notice as soon as customer wanted to change a phone number there. Another widget with an <img> tag and some text suddenly had added <p> tags, which broke the layout. We stopped updating our sites to 4.8 immediately after this, even rolled back some.

Many of our customers use text widgets with all kind of html content.

Manually copying these "since ever existing" widgets to a new "Custom HTML" widget is not an option for us.

Many widgets also have complex "Widget Logic" settings, would be a additional nightmare (=impossible) to copy these to the correct new widgets.

Regarding your sceenshot:

What should a regular customer (not wp expert) do, if he reads this "Custom HTML" widget message? He/she is happy to find that place where to change the phone number or edit the latest special offer text. He/she does not want be bothered with any technical notes.

Can the note be dismissed per user? Is there an action to notify wp admin if this message appears or a filter to stop this message from appearing at all and keeping all text widgets as before?

#124 in reply to: ↑ 123 @westonruter
7 years ago

Replying to Ov3rfly:

Manually copying these "since ever existing" widgets to a new "Custom HTML" widget is not an option for us.
Many widgets also have complex "Widget Logic" settings, would be a additional nightmare (=impossible) to copy these to the correct new widgets.

You won't be forced to copy the content of the legacy Text widgets to Custom HTML. Once a Text widget is in legacy mode it will remain in legacy mode. However, when creating new widgets, you will use the Custom HTML widget instead of the Text widget, as newly created widgets are not initialized in legacy mode.

Can the note be dismissed per user?

It is not currently dismissible notice. That's an option, but if the notice is not there then users may be confused about why some Text widgets have visual editing while others do not. The presence of the notice clarifies why a given Text widget lacks the visual editor.

Is there an action to notify wp admin if this message appears or a filter to stop this message from appearing at all and keeping all text widgets as before?

There is no filter to hide the message, but you could hide it with CSS.

#125 @Ov3rfly
7 years ago

Please add a filter to hide that message. To be used like this.

Adding custom css to a hundred different theme admin areas is also not an option, as it needs testing.

Thanks for your quick feedback.

Last edited 7 years ago by Ov3rfly (previous) (diff)

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


7 years ago

#127 follow-up: @Ov3rfly
7 years ago

A closer look shows another? problem in 4.8 code. Pre-existing html-comment gets wrapped in <p> when printed to front end, which leads to extra empty space in front end. Also extra linebreaks are added within the <p class="some-class>

Pre-existing HTML in text widget, stays like this in 4.8:

<p class="some-class">Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.</p>
<!-- some comment "lorem ipsum" -->

view-source of front end, with 4.8:

<p class="some-class">
Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.
</p>
<p><!-- some comment "lorem ipsum" --></p>

Visitor sees extra empty space due to "non-empty" <p>

#128 in reply to: ↑ 127 @westonruter
7 years ago

Replying to Ov3rfly:

A closer look shows another? problem in 4.8 code. Pre-existing html-comment gets wrapped in <p> when printed to front end, which leads to extra empty space in front end. Also extra linebreaks are added within the <p class="some-class>

I presume you did not have the auto-paragraphing checkbox checked in this example? Please try editing a widget with those contents when the patch is applied to your install. I just tried, and I see it is presented in legacy mode. See also the PR.

I may need to add the detection of HTML comments to also opt-in to legacy mode.

#129 follow-up: @Ov3rfly
7 years ago

Before 4.8 update that checkbox was not checked, after 4.8 update there was no more checkbox.

We can't apply any patches to this website, it's rolled back already.

We won't update any website to 4.8.x until this widget mess is solved in a new release.

Rolling back websites before customers could start to mess up widgets by just editing a phone number was enough work...

#130 in reply to: ↑ 129 @westonruter
7 years ago

Replying to Ov3rfly:

We can't apply any patches to this website, it's rolled back already.

Of course. I didn't mean to test this in a live production environment. Any testing you can do on a staging or dev environment would be helpful to ensure your update to 4.8.1 will be smooth.

#131 @dougal
7 years ago

Just did some testing and everything seemed to behave as expected:

  • Set up WP 4.7 and created text widgets with html content known to have problems in 4.8.0.
  • Upgraded to trac-40951 branch. Text widgets gave the legacy mode warning, but still rendered "correctly" (old behavior)
  • Tried toggling "Automatically add paragraphs", re-saving, reviewing. Behaved as expected (same as in 4.7). Toggling back off and re-saving restored desired rendering.
  • Creating a new text widget and pasting in the identical content did not trigger legacy mode. The wpautop filter was applied, giving "broken" rendering (relative to 4.7), as expected.
  • Pasting HTML into the WYSIWYG tab triggered friendly warning that I probably wanted the Text tab.

Thumbs-up from me.

#132 @westonruter
7 years ago

  • Keywords commit added

#133 @westonruter
7 years ago

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

In 41050:

Widgets: Add legacy mode for Text widget and add usage pointers to default visual mode.

The Text widget in legacy mode omits TinyMCE and retains old behavior for matching pre-existing Text widgets. Usage pointers added to default visual mode appear when attempting to paste HTML code into the Visual tab and when clicking on the Text tab, informing users of the new Custom HTML widget.

Props westonruter, melchoyce, gitlost for testing, obenland for testing, dougal for testing, afercia for testing.
See #35243.
Fixes #40951.

#134 @westonruter
7 years ago

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

Re-opening for 4.8.1

#135 @westonruter
7 years ago

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

In 41053:

Widgets: Add legacy mode for Text widget and add usage pointers to default visual mode.

The Text widget in legacy mode omits TinyMCE and retains old behavior for matching pre-existing Text widgets. Usage pointers added to default visual mode appear when attempting to paste HTML code into the Visual tab and when clicking on the Text tab, informing users of the new Custom HTML widget.

Merges [41050] to 4.8 branch.
Props westonruter, melchoyce, gitlost for testing, obenland for testing, dougal for testing, afercia for testing.
See #35243.
Fixes #40951 for 4.8.1.

#136 follow-up: @bobbingwide
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Comments on the new Custom HTML code... having tested the new solution for #40993.

  1. I don't understand why is_legacy_instance() is called during wp_list_widgets() processing. Note: It doesn't cause a problem at present.
  1. I think that the message that says 'This widget contains code that may work better in the new “Custom HTML” widget. How about trying that widget instead?' doesn't need the word 'new'. Some time in the future new will be old.
  1. When the widget text contains a shortcode then it may not work better in the Custom HTML widget as do_shortcode is not automatically attached to 'widget_custom_html_content'. So, until logic is added to enable shortcode processing it'll be worse.
  1. The message doesn't go away even when the widget has been changed.

So perhaps the message should say 'This widget may contain code...'.

  1. And what's meant by 'code' anyway. Some users may think it's the code tag(s) created by the code button.
  1. Checking for the presence of a shortcode assumes that the shortcode has been registered. Not all shortcodes are registered during admin processing. Sometimes shortcodes are inactive since the plugins to which they're associated have been deactivated.
  1. Does the presence of shortcodes matter anyway, now that there's just in time processing?

This ticket was mentioned in Slack in #core by bobbingwide. View the logs.


7 years ago

#138 in reply to: ↑ 136 ; follow-ups: @westonruter
7 years ago

Replying to bobbingwide:

Comments on the new Custom HTML code... having tested the new solution for #40993.

Many thanks for testing.

  1. I don't understand why is_legacy_instance() is called during wp_list_widgets() processing. Note: It doesn't cause a problem at present.

The reason is that the legacy mode is checked when outputting the widget controls, when the form method is invoked. The form method then will then decide whether or not the widget controls should be presented in legacy mode or not, and once a widget is saved in legacy mode or in non-legacy mode, then the results will get stored in the widget instance as the legacy=true prop or the filter=content prop, respectively. You can see that the widget method looks at the filter prop to see if has been set to 'content', and if so then that indicates that the new filter widget_text_content filter should apply.

  1. I think that the message that says 'This widget contains code that may work better in the new “Custom HTML” widget. How about trying that widget instead?' doesn't need the word 'new'. Some time in the future new will be old.

I think we can remove the “new” word in trunk but it should be left in the 4.8 branch. @melchoyce agreed?

  1. When the widget text contains a shortcode then it may not work better in the Custom HTML widget as do_shortcode is not automatically attached to 'widget_custom_html_content'. So, until logic is added to enable shortcode processing it'll be worse.

Shortcodes aren't automatically attached to widget_text_content filter either, or widget_text for that matter. A plugin has to opt-in to do shortcode processing in widgets to begin with, so a plugin would need to do the same for the Custom HTML widget.

  1. The message doesn't go away even when the widget has been changed.

So perhaps the message should say 'This widget may contain code...'.

Good point. Or even, “This widget may have contained code”. @melchoyce thoughts?

  1. And what's meant by 'code' anyway. Some users may think it's the code tag(s) created by the code button.

So maybe it should say “custom HTML” instead of “code”?

  1. Checking for the presence of a shortcode assumes that the shortcode has been registered. Not all shortcodes are registered during admin processing. Sometimes shortcodes are inactive since the plugins to which they're associated have been deactivated.

You mean in the is_legacy_instance method? Actually, that whole bit of logic could in reality be reverted. You see, the commit 1ffb451 was added first, and then a more robust method for temporarily moving the do_shortcode handler was done subsequently in 6e6b53d. So there isn't actually a need to enter into legacy mode when a preexisting widget contains a shortcode.

  1. Does the presence of shortcodes matter anyway, now that there's just in time processing?

Hah. Yes, exactly! I can revert 1ffb451 then, unless anyone sees a reason for keeping it around.

#139 follow-up: @Ov3rfly
7 years ago

  1. Please notify all plugin/theme authors who add shortcode processing for text widgets about this requirement.
  1. If a shortcode outputs HTML comments, the layout in front end is broken with 4.8 due to unwanted added <p> tags, see above.
Last edited 7 years ago by Ov3rfly (previous) (diff)

#140 in reply to: ↑ 138 @bobbingwide
7 years ago

Replying to westonruter:

  1. I don't understand why is_legacy_instance() is called during wp_list_widgets() processing. Note: It doesn't cause a problem at present.

The reason is that the legacy mode is checked when outputting the widget controls, when the form method is invoked. The form method then will then decide whether or not the widget controls should be presented in legacy mode or not, and once a widget is saved in legacy mode or in non-legacy mode, then the results will get stored in the widget instance as the legacy=true prop or the filter=content prop, respectively. You can see that the widget method looks at the filter prop to see if has been set to 'content', and if so then that indicates that the new filter widget_text_content filter should apply.

I was referring to its invocation when displaying the Text widget in Available widgets, but now I see that the same logic's also used for Inactive widgets. So, I understand it's a necessary evil.

#141 in reply to: ↑ 138 ; follow-up: @bobbingwide
7 years ago

Replying to westonruter:

  1. The message doesn't go away even when the widget has been changed.

So perhaps the message should say 'This widget may contain code...'.

Good point. Or even, “This widget may have contained code”. @melchoyce thoughts?

Is there any reason why legacy mode has to be permanent?

Some widgets could become legacy simply because of a new line at the end of the text.
Removal of that new line would allow use of the visual editor.

And conversely, any reason why $instance['filter'] = 'content' is also permanent?
A flexible approach would allow a user to switch to text only while still giving the option of wpautop processing.

It may also cater for users who upgraded to 4.8 and now have problematic widgets.

Last edited 7 years ago by westonruter (previous) (diff)

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


7 years ago

#143 @FolioVision
7 years ago

To point out the obvious, what's the issue with just turning "legacy" text widgets into what you now call HTML Widgets? The logic of trying to force people to use wpautop and TinyMCE on widgets which just weren't mean for TinyMCE and all of its troublesome filters seems completely bizarre to me.

Anyone who wants to move HTML widgets over to Text widgets can do so at his/her leisure. Nothing breaks. The logic of the widgets remains a lot simpler.

#144 @westonruter
7 years ago

@FolioVision I believe the rationale for this is largely contained in my comment above. It's the second proposal I described there:

Another solution proposed was to revert the Text widget back to its original state (maybe renaming to HTML Code widget) and introduce a new Rich Text widget. However, a key reason for letting the existing Text widget remain upgraded with TinyMCE is that novice users who previously have just put simple text into the widget should by default get the new visual editing experience. Novice users shouldn't have to be bothered with copy/pasting the contents of their widget into a separate widget in order to start doing visual editing, in the same way that they don't have to switch between post editing experiences.

I think another (maybe) unspoken goal here is to kill off the “wpautop” checkbox. With what is now committed to trunk, users will still be able to move content from Text widgets over to Custom HTML widgets at their leisure, due now to the legacy mode.

#145 in reply to: ↑ 139 ; follow-up: @westonruter
7 years ago

  • Keywords needs-dev-note added

Replying to Ov3rfly:

  1. Please notify all plugin/theme authors who add shortcode processing for text widgets about this requirement.

Good idea. I'll work on that by slurping down all plugins and ack for instances of shortcode handling being added to widgets. We should also publish a new dev-note that introduces the Custom HTML widget and the legacy mode for the Text widget.

  1. If a shortcode outputs HTML comments, the layout in front end is broken with 4.8 due to unwanted added <p> tags, see above.

This should not happen because 6e6b53d moves do_shortcode from widget_text to widget_text_content after wpautop will have already applied. Please test to confirm.

#146 @melchoyce
7 years ago

Let's go with "may contain code"

#147 in reply to: ↑ 141 ; follow-up: @westonruter
7 years ago

Replying to bobbingwide:

Some widgets could become legacy simply because of a new line at the end of the text.

Great point. I've just re-opened the PR to account for that, by trimming whitespace before checking for line breaks: 56c9456.

Is there any reason why legacy mode has to be permanent?
Removal of that new line would allow use of the visual editor.
A flexible approach would allow a user to switch to text only while still giving the option of wpautop processing.
It may also cater for users who upgraded to 4.8 and now have problematic widgets.

The reason why the legacy mode becomes permanent is in large part due to how widgets are implemented as PHP animals. Whenever you save a widget, it re-calls the form method and sends it back in the Ajax response to replace the previous form. The side effect of this is that any form elements that are constructed with JS will get destroyed every time an update is made. The new TinyMCE Text widget (and the media widgets as well) work around this by moving the DOM container for the fields outside of the element that gets replaced with each save. The Text widget in legacy mode, however, does not do this: it uses the old approach of replacing the form fields. All this to say, there would be a lot more complexity (not to mention user disorientation) to try to switch between legacy and visual interactively while the user is making changes, especially in the Customizer where there is no “Save” button.

And conversely, any reason why $instance['filter'] = 'content' is also permanent?

Since the wpautop checkbox is removed entirely going forward, the filter property becomes useless and it here essentially becomes a versioning tool, indicating that a Text widget was created in 4.8 or later.

#148 follow-up: @FolioVision
7 years ago

@westonruter Thanks for your note. I've gone back to the referenced post for the pertinent sections.

This will not have the desired effect for some advanced users because it will change the widget's ID that contains the content, and this will break CSS selectors that target the widget. It can also have negative impacts for plugins that have existing references to Text widget IDs for placement in contexts outside normal dynamic_sidebar() calls.

So why not then just keep the Widget ID for the converted widget.

Even for non-advanced users, migrating existing Text widgets over to HTML Code widgets is not ideal because then novice users, who may not know HTML to begin with, will have difficulty locating the widget that has the content they need to modify. Seeing the word “HTML” may also confuse and intimidate them.

Wow. How the mighty have fallen. When the word HTML confuses WordPress site admins we've completely lost the thread (quick reminder, WordPress is used to build websites, websites are based on HTML). So from a developer friendly platform, we are breaking people's sites and building breaking upgrades so no one with a fancy existing HTML widget has to see the word HTML.

This seems very, very wrong to me. What am I missing?

another (maybe) unspoken goal here is to kill off the “wpautop” checkbox. With what is now committed to trunk, users will still be able to move content from Text widgets over to Custom HTML widgets at their leisure, due now to the legacy mode.

Why should people have to go through painful manual moves when we could just do it for them automatically and seamlessly. That's what computers are for Weston, lightening the burden of pointless manual labour.

Last edited 7 years ago by FolioVision (previous) (diff)

#149 in reply to: ↑ 148 ; follow-up: @westonruter
7 years ago

Replying to FolioVision:

This will not have the desired effect for some advanced users because it will change the widget's ID that contains the content, and this will break CSS selectors that target the widget. It can also have negative impacts for plugins that have existing references to Text widget IDs for placement in contexts outside normal dynamic_sidebar() calls.

So why not then just keep the Widget ID for the converted widget.

Widget IDs are different from post IDs. Given how widgets are (unfortunately) stored in a single serialized array in options, there is no auto-incremented number from MySQL. The number part of a widget's ID is the index in the array. Then in order to disambiguate the number from other widget types, a widget ID is prefixed by the widget type (id_base) for example text-123. So converting such a widget over to Custom HTML would not only change the type prefix but it would also get a different array index, both breaking any CSS selector.

See also #35669 where widgets are proposed to be stored in a custom post type, so they would get an auto-incremented post ID. This will probably be implemented as part of blocks in Gutenberg.

Why should people have to go through painful manual moves when we could just do it for them automatically and seamlessly. That's what computers are for Weston, lightening the burden of pointless manual labour.

Per above, we can't automatically move them as the migration would result in ID changes.

#150 in reply to: ↑ 149 ; follow-up: @FolioVision
7 years ago

Thanks for sharing your thought process, Weston.

Replying to westonruter:

So why not then just keep the Widget ID for the converted widget.

Widget IDs are different from post IDs. Given how widgets are (unfortunately) stored in a single serialized array in options, there is no auto-incremented number from MySQL. The number part of a widget's ID is the index in the array. Then in order to disambiguate the number from other widget types, a widget ID is prefixed by the widget type (id_base) for example text-123. So converting such a widget over to Custom HTML would not only change the type prefix but it would also get a different array index, both breaking any CSS selector.

It seems to me the existing widget could remain in place with a flag to open it in the HTML widget, i.e. effectively it is an HTML widget.

See also #35669 where widgets are proposed to be stored in a custom post type, so they would get an auto-incremented post ID. This will probably be implemented as part of blocks in Gutenberg.

Another dubious idea, I remember when this came up. I believe it was Iseulde's idea. Widgets are not mainly posts. They are *widgets* and hence should store javascript, PHP and HTML very easily. I.e. free form content Trying to store widgets in post content is just pushing square pegs into round holes.

Why should people have to go through painful manual moves when we could just do it for them automatically and seamlessly. That's what computers are for Weston, lightening the burden of pointless manual labour.

Per above, we can't automatically move them as the migration would result in ID changes.

I'm sorry Weston, but it sounds for carefully formulated excuses for a lack of respect for both user and agency time. I can't imagine how our clients would react if we just broke their sites for the heck of it. That's what we (collectively) are doing.

#151 in reply to: ↑ 145 ; follow-ups: @Ov3rfly
7 years ago

Replying to westonruter:

Replying to Ov3rfly:

  1. Please notify all plugin/theme authors who add shortcode processing for text widgets about this requirement.

Good idea. I'll work on that by slurping down all plugins and ...

Don't forget all themes, in theme repository, and all commercial sellers, and all abandoned plugins, and all abandoned themes, and also all private coders... well, no. Do not notify anybody. It is simply not possible The initial suggestion was made to hopefully lead you to this conclusion.

  1. If a shortcode outputs HTML comments, the layout in front end is broken with 4.8 due to unwanted added <p> tags, see above.

This should not happen because ...

It is happening now, with a 4.8 updated site and a shortcode which outputs a <ul> with a few <li> and one "commented" <li> which is printed as <p> now, which is bad, very bad, and required code changes within the shortcode PHP-Code. We don't have time to look at any future patches, we have to deal with customers who updated to 4.8 and all their broken widgets at the moment.

You can't expect any/all WordPress users to (even be able to) transfer all their widgets to other widgets just because somebody had the tiny bad idea to change a "since ever existing" core widget to something else instead of adding a new widget for a completely new feature. People now are upset about WordPress already because their "site looks different" and they "did not do anything" and they have to pay hourly rates to get their sites fixed. Even if they somehow will manage it alone, to copy that "whatever it is" shortcode to a new HTML widget, the shortcode won't even work there because the widget_text/do_shortcode filter is not there, just imagine that disappointment.

Sorry about this rant, nothing personal, a real and clean solution would be a complete rollback of "text widget" to pre-4.8 as soon as possible and add a new "tinymce widget" for future use.

Last edited 7 years ago by Ov3rfly (previous) (diff)

#152 in reply to: ↑ 151 @fullworks
7 years ago

Having taken the time to write plugin that reverts the text widget back to pre-4.8 functionality to solve those problems listed by so many, (https://wordpress.org/plugins/add-paragraphs-option-to-text-widget/) without modifying core files, my main concern is for the users of that plugin that I will be able to continue to revert after 4.8.1. I see the new code is in trunk, I haven't tested yet the ability to revert that, but will take a look soon.

#153 in reply to: ↑ 150 @westonruter
7 years ago

Replying to FolioVision:

It seems to me the existing widget could remain in place with a flag to open it in the HTML widget, i.e. effectively it is an HTML widget.

Yes, but then there'd essentially be one widget that is overloaded to have two separate purposes: showing content and showing HTML code. We're trying to separate out these into the two separate widgets with the introduction of the separate Custom HTML widget. So that is why the legacy mode of the Text widget cannot be entered into for new instances.

Alternatively, an idea would be to have a way to give a UI for the user to opt-in to transform a Text widget into a Custom HTML widget. The closest we got to that can be seen in add-widget-link-auto-search-available-widgets-panel.png where Add Widget link causes the available widgets panel to open with the “Custom HTML” being pre-searched for. This feature, however, is only available in the Customizer given that there is a lack of any such construct in the widgets admin screen. In general, widgets lack a proper data model and JavaScript API to accomplish this widget transformation from a Text widget to a Custom HTML widget, something which is being accounted for from the ground up in the development of blocks in Gutenberg. So while it would be technically possible to implement logic for doing the transformation from a Text block to a Custom HTML block in the UI, the complexity required to implement given the current state of widgets would lead to diminishing returns, I feel.

Widgets are not mainly posts. They are *widgets* and hence should store javascript, PHP and HTML very easily. I.e. free form content Trying to store widgets in post content is just pushing square pegs into round holes.

I think we have different understandings of what a “post” is. I agree that widgets are not mainly articles, but data. Custom post types are being used to store JSON data today in the customize_changeset post type to store changesets in the Customizer, and they are being used to store Additional CSS in the custom_css post type. We're also considering using a private custom post type to store oEmbed caches in #34115. The post type is WordPress's scalable storage model for objects, whether that be article content or data content. Using a custom post type to store JSON data containing a given widget/block's instance data seems natural to me. Anyway, this is going off topic.

I'm sorry Weston, but it sounds for carefully formulated excuses for a lack of respect for both user and agency time. I can't imagine how our clients would react if we just broke their sites for the heck of it. That's what we (collectively) are doing.

Nothing will be broken because existing Text widgets will be in legacy mode if the expansive conditions are met. Also, I work at an agency that specializes in WordPress, so I very much respect agency time. I do not work for Automattic and I don't work on WordPress.com. I'm very much concerned for not breaking sites and that is why I've worked out this legacy mode, so existing installs will continue to work as they have been, but then moving forward users will start transitioning to use separate widgets depending on whether they want to write content (Text widget) or code (Custom HTML widget). As I've said before, I don't think this legacy mode is an excuse or an attempt to save face: I would have chosen this route from the beginning for 4.8 if these issues had arisen during testing.

I feel like we're re-hashing the same things over and over. In the end we'll just have to agree to disagree.

#154 in reply to: ↑ 151 @westonruter
7 years ago

Replying to Ov3rfly:

Don't forget all themes, in theme repository, and all commercial sellers, and all abandoned plugins, and all abandoned themes, and also all private coders... well, no. Do not notify anybody. It is simply not possible The initial suggestion was made to hopefully lead you to this conclusion.

We could potentially add the do_shortcode function to the widget_custom_html_content filter if it is currently added to the widget_text filter. See custom-html-widget-shortcode-support.diff.

#155 follow-up: @westonruter
7 years ago

Or actually, we could just go ahead and apply the widget_text filters on the Custom HTML content. See #40907 (comment 61).

#156 @westonruter
7 years ago

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

In 41070:

Widgets: Discount shortcodes and leading/trailing line breaks for triggering Text widget legacy mode.

Since plugin-added shortcode handling is just-in-time deferred to run after wpautop, there is no need to retain the presence of shortcodes to trigger legacy mode. Also updates Text widget legacy mode notice informing of Custom HTML widget.

Amends [41050].
Props westonruter, melchoyce.
Fixes #40951 for trunk.

#157 @westonruter
7 years ago

In 41071:

Widgets: Discount shortcodes and leading/trailing line breaks for triggering Text widget legacy mode.

Since shortcode handling is just-in-time deferred to run after wpautop, there is no need to retain the presence of shortcodes to trigger legacy mode. Also updates Text widget legacy mode notice informing of Custom HTML widget.

Merges [41070] onto 4.8 branch.
Amends [41050].
Props westonruter, melchoyce.
Fixes #40951 for 4.8.1.

#158 in reply to: ↑ 147 ; follow-up: @bobbingwide
7 years ago

Replying to westonruter:

And conversely, any reason why $instance['filter'] = 'content' is also permanent?

I think the code should determine the value for legacy during save and then make allowances for it when deciding what filters to apply when formatting. If 'legacy' has been set then don't apply wpautop.

In this scenario, when displaying the form for the text widget, 'content' trumps 'legacy'. BTW. I don't particularly care for the term 'legacy'. Why not try something like 'complex' or 'mixed'?

#159 in reply to: ↑ 155 @bobbingwide
7 years ago

Replying to westonruter:

Or actually, we could just go ahead and apply the widget_text filters on the Custom HTML content. See #40907 (comment 61).

You mean apply balancetags? If used at all this should be applied during save.

#160 in reply to: ↑ 158 @westonruter
7 years ago

Replying to bobbingwide:

I think the code should determine the value for legacy during save and then make allowances for it when deciding what filters to apply when formatting. If 'legacy' has been set then don't apply wpautop.

In this scenario, when displaying the form for the text widget, 'content' trumps 'legacy'.

The reason for having both a filter=content property and a legacy=1 property are for backwards-compatibility and because we can only store a boolean value in filter when in legacy mode. When filter is set to content, it already knows it is not in legacy mode because legacy mode would only ever store a boolean value in the property. In the visual editor, the filter property is not used and is only set to content to indicate essentially that the the “content” filters apply.

BTW. I don't particularly care for the term 'legacy'. Why not try something like 'complex' or 'mixed'?

It's legacy because it is only there for backwards-compatibility. New installs will never encounter it. It will be like Links in WordPress: it's still there, but it is only shown if an install has been around for a long time.

You mean apply balancetags? If used at all this should be applied during save.

Let's continue this on #40907. I've replied to you about that at https://core.trac.wordpress.org/ticket/40907#comment:65

This ticket was mentioned in Slack in #core by wido. View the logs.


7 years ago

#162 @melchoyce
7 years ago

#41360 was marked as a duplicate.

#163 @westonruter
7 years ago

In 41086:

Widgets: Replace adding balanceTags on widget_custom_html_content filter in favor of just applying widget_text filters in the Custom HTML widget.

Ensures that users who copy HTML from the Text widget in legacy mode over to the Custom HTML widget will continue to get all of the same filters applied, including tag balancing and shortcodes, if a plugin added support. Plugins still have the widget_text_content and widget_custom_html_content filters they can use to target the specific widget types.

Amends [40893].
See #40951.
Fixes #40907 for trunk.

#164 @westonruter
7 years ago

In 41087:

Widgets: Replace adding balanceTags on widget_custom_html_content filter in favor of just applying widget_text filters in Custom HTML widget.

Ensures that users who copy HTML from the Text widget in legacy mode over to the Custom HTML widget will continue to get all of the same filters applied, including tag balancing and shortcodes, if a plugin added support. Plugins still have the widget_text_content and widget_custom_html_content filters they can use to target the specific widget types.

Merges [41086] onto 4.8 branch.
Amends [40893].
See #40951.
Fixes #40907 for 4.8.1.

#165 @westonruter
7 years ago

All: WordPress 4.8.1-beta1 is now available for testing.

As announced, please help us test by running the WordPress Beta Tester plugin on your test sites. You can read more about testing in the Core Handbook: https://make.wordpress.org/core/handbook/testing/beta/

See some testing instructions above at https://core.trac.wordpress.org/ticket/40951#comment:112

What is important is to test the upgrade on a site on 4.7.x that contains existing Text widgets, particularly any that have custom HTML that 4.8 would have munged.

For manual updates, a ZIP can also be found under https://wordpress.org/download/release-archive/#beta-and-rc with a direct link to the ZIP: https://wordpress.org/wordpress-4.8.1-beta1.zip

For any issues that arise (which hopefully there are none) please open separate tickets in Trac, as this ticket is too heavy.

Thanks.

#166 @westonruter
7 years ago

Thanks to your testing and the testing from users on the AWP Facebook group, some additional fixes are being made, including:

  • #41392: Theme styles for Text widget do not apply to Custom HTML widget (committed to trunk)
  • #41386: Text Widget - Wording - Legacy Mode 4.8.1 beta
  • #41394: Text widget: Rename legacy mode to visual mode and improve back-compat for widget_text filters

This last one in particular I'd appreciate your review on. In particular, @bobbingwide, the patch on #41394 partly addresses your feedback in comment 158. Please leave any feedback on these tickets and not this one. Thanks!

#167 @westonruter
7 years ago

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.

#168 @westonruter
7 years ago

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.

#169 @westonruter
7 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.

#170 @westonruter
7 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.

#171 @westonruter
7 years ago

All: WordPress 4.8.1-RC1 is now available for testing.

As announced and I commented above, please help us test by running the WordPress Beta Tester plugin on your test sites. You can read more about testing in the Core Handbook: https://make.wordpress.org/core/handbook/testing/beta/

For manual updates, a ZIP can also be found under https://wordpress.org/download/release-archive/#beta-and-rc with a direct link to the ZIP: https://wordpress.org/wordpress-4.8.1-RC1.zip

As before, please report any issues in new tickets. Thank you!

#172 @fullworks
7 years ago

Hi I'm adding to this trac, as this is just an observation rather than an issue, hope that is OK.

I have just completed building a complete site using a Genesis child theme and 4.8.1 beta. Everything is operating well, as long as the 'Custom HTML' widget is substituted when previously the 'Text Widget' is advised in the theme documentation. So I think some sort of communication to theme designers ( especially Studio Press ) to update their documentation would be well advised.

From my perspective, as a site developer that uses Genesis a lot, the Text Widget now is pretty much of no use at all and the go to widget has to be Custom HTML for 99% of the tasks I need.

#173 @FolioVision
7 years ago

From my perspective, as a site developer that uses Genesis a lot, the Text Widget now is pretty much of no use at all and the go to widget has to be Custom HTML for 99% of the tasks I need.

It does really seem vicious to break an existing widget rather than add a new one.

What's worse is this overwrought hybrid mode which way overcomplicates handling HTML widgets. If the content fails the text widget tests, it should immediately be converted to an HTML widget.

The end result for the user is much cleaner. He has his widget in a useable format with no drama. I'm sure the internal routing would be much easier than managing hybrid mode.

It feels like the people who conceived this and the people who implemented it have not built websites in earnest in a very, very long time.

#174 @westonruter
7 years ago

In 41251:

Widgets: Suppress PHP warnings raised by DOMDocument::loadHTML() in WP_Widget_Text::is_legacy_instance() which could appear in Text widget forms.

Also explicitly use HTML5 doctype when parsing Text widget contents in legacy mode detection.

Amends [41050].
See #40951.
Fixes #41622.

#175 @ocean90
7 years ago

In 41391:

Widgets: Suppress PHP warnings raised by DOMDocument::loadHTML() in WP_Widget_Text::is_legacy_instance() which could appear in Text widget forms.

Also explicitly use HTML5 doctype when parsing Text widget contents in legacy mode detection.

Merge of [41251] to the 4.8 branch.

Amends [41050].
See #40951.
Fixes #41622.

#176 @westonruter
6 years ago

#44417 was marked as a duplicate.

#177 @desrosj
4 years ago

  • Keywords has-dev-note added; commit needs-dev-note removed
Note: See TracTickets for help on using tickets.