WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#19925 closed defect (bug) (fixed)

Empty widget title in admin not handled properly

Reported by: kawauso Owned by: azaozz
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

Steps to reproduce:

  1. Add a widget
  2. Enter a title and save
  3. Clear the title and save
  4. The title in the widget handle will stay set to the original title value

Line 252 of wp-admin/js/widgets.dev.js checks whether title.val() is a valid value, but this will also reject empty strings. Attached patch checks rejects for undefined values and clears the widget's handle title if the string is just empty.

Attachments (2)

19925.diff (633 bytes) - added by kawauso 4 years ago.
typeof check rejected values
19925.1.diff (583 bytes) - added by mp3j3rk 3 years ago.
patches ./wp-admin/js/widgets.js

Download all attachments as: .zip

Change History (9)

@kawauso4 years ago

typeof check rejected values

comment:1 follow-up: @mp3j3rk3 years ago

First off, I'm still new, so I could be completely off-base here.

Before patching, after a refresh, or navigating away from the page and then back to it, the name updates. Obviously, we would like to see the widget's title update immediately after clicking Save.

After applying the patch, I did not see any change to the behavior.

I do have a question, on line 256 of the patched file, why did you use

if (typeof title != "undefined")

instead of something like

if (typeof title == "undefined" || title === null || title === '')

?

comment:2 in reply to: ↑ 1 @kawauso3 years ago

Replying to mp3j3rk:

Before patching, after a refresh, or navigating away from the page and then back to it, the name updates. Obviously, we would like to see the widget's title update immediately after clicking Save.

That's rather odd. It should be being updated along with the widget saving spinner in either case. Perhaps there's a JS conflict on your site or JS is disabled?

why did you use

if (typeof title != "undefined")

instead of something like

if (typeof title == "undefined" || title === null || title === '')

The initial

if ( title = title.val() )

will be true for any case where title is a non-empty string, so

if (typeof title != "undefined")

just sorts between the two other potential values returned by jQuery.val(), an empty string and an undefined value when there is no title field. In the latter case, we don't want to touch the title at all since there is no title value to display (rather than an empty one).

comment:3 @mp3j3rk3 years ago

The issue looks to me as though changes to widgets.dev.js are not reflected in the tests for this bug. I was able to trace the issue back into widgets.js.

Unfortunately, the code in this file showed up on a single line for me (using linux, I suspect the file was using windows newlines). I was able to re-beautify it and then make a diff patch from it.

It's pretty much the same idea as what kawauso had with the inital patch, but its in a different file.

The diff patch is attached as 19925.1.diff

@mp3j3rk3 years ago

patches ./wp-admin/js/widgets.js

comment:4 @dd323 years ago

The issue looks to me as though changes to widgets.dev.js are not reflected in the tests for this bug. I was able to trace the issue back into widgets.js.

They're the "Same" file.

.js files are minified versions of .dev.js files.

To debug JS/css you add SCRIPT_DEBUG to your wp-config.php file, this will cause WordPress to use the .dev.css and .dev.js files instead, this allows you to debug it properly and understand the code you're looking at.

comment:5 @mp3j3rk3 years ago

Even with debug set to true, I do not see any change when adding the original patch to the .dev.js file. I still only see change when widgets.js is modified.

I'm running wordpress 3.3 from the svn branch and installed locally as a separate instance from my other local installs. Testing in firefox aurora v12.0a2, and firefox 10. Not sure why it wouldn't be using the .dev files since debug is true.

comment:6 @mp3j3rk3 years ago

I figured out what my deal was. SCRIPT_DEBUG was not set to true like I had thought it was. After setting it in wp-config.php, everything looks good with the original patch.

Confirming patch 19925.diff on Wordpress v3.3.1 svn revision 20101 with firefox aurora v12.0a2.

comment:7 @azaozz3 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [20628]:

Widgets: remove the widget title shown in the widget header when empty or doesn't exist, part props kawauso, fixes #19925

Note: See TracTickets for help on using tickets.