Opened 9 years ago
Closed 9 years ago
#19925 closed defect (bug) (fixed)
Empty widget title in admin not handled properly
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | 3.3.1 |
Component: | Administration | Keywords: | has-patch |
Focuses: | Cc: |
Description
Steps to reproduce:
- Add a widget
- Enter a title and save
- Clear the title and save
- 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)
Change History (9)
#1
follow-up:
↓ 2
@
9 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 === '')
?
#2
in reply to:
↑ 1
@
9 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).
#3
@
9 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
#4
@
9 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.
#5
@
9 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.
typeof check rejected values