WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#7509 closed defect (bug) (fixed)

Fix up some Notices in admin

Reported by: DD32 Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.7
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

Attached patch silences a lot of errors with the adition of isset()'s and use of temporary variables.

A special note needs to be given to wp-includes/taxonomy.php, A 'return null' was replaced with a WP_Error return value, The function allready returns a WP_Error on the next line for a different error condition, so it shouldnt break any code, But it may need to be replaced with a null again.

Affected code is about line 300:

	if ( empty($term) ) {
		return null;

Replaced with:

	if ( empty($term) ) {
		$error = new WP_Error('invalid_term', __('Empty Term'));
		return $error;
	}

Could also be replaced with:

	if ( empty($term) ) {
		$error = null;
		return $error;
	}

(Only variables can be returned by reference, returning null or new WP_Error directly causes a notice)

Attachments (21)

7509.2.diff (1.5 KB) - added by DD32 6 years ago.
notice in generate_tag_cloud() & attempt at getting rid of the cookiehash message
7509.r8877.diff (14.9 KB) - added by jacobsantos 6 years ago.
Fix wp-admin notices based off of r8877
7509.3.diff (1.1 KB) - added by DD32 6 years ago.
admin-notices.r8976.diff (2.6 KB) - added by jacobsantos 6 years ago.
More corrections that didn't make the last cut. based off of r8976
7509.4.diff (530 bytes) - added by DD32 6 years ago.
7509.5.diff (572 bytes) - added by DD32 6 years ago.
7509.6.diff (1.9 KB) - added by DD32 6 years ago.
includes 7509.5.diff
7509.7.diff (1.6 KB) - added by DD32 5 years ago.
7509.diff (1.9 KB) - added by DD32 5 years ago.
7509.8.diff (449 bytes) - added by DD32 5 years ago.
ensure ->response is set on update_plugins option to prevent menu notice.
7509.9.diff (867 bytes) - added by DD32 5 years ago.
2008-11-02.notices.diff (5.8 KB) - added by filosofo 5 years ago.
7509.patch (11.3 KB) - added by Viper007Bond 5 years ago.
Superceeds filosofo's patch (i.e. includes their changes)
7509.2.patch (1.5 KB) - added by Viper007Bond 5 years ago.
7509.10.diff (14.5 KB) - added by DD32 5 years ago.
7509.11.diff (3.7 KB) - added by DD32 5 years ago.
7509.12.diff (2.0 KB) - added by DD32 5 years ago.
7509.13.diff (8.2 KB) - added by DD32 5 years ago.
7509.14.diff (15.1 KB) - added by DD32 5 years ago.
7509.15.diff (2.9 KB) - added by DD32 5 years ago.
7509.15.2.diff (2.8 KB) - added by DD32 5 years ago.

Download all attachments as: .zip

Change History (71)

comment:1 DD326 years ago

Also, wp-admin/edit.php has some dead code in it, Which i commented out, I'm not sure if that needs to be kept and fixed up, or discarded all together. (See first chunk in the Diff)

Also, wp-includes/capabilities.php change added first and last names only as they can be commonly empty, I'm not sure if any other members which are added should also be prefilled to empty, However, Someone may want to take a closer look at that edit to see if it needs more default properties for the other fields.

comment:2 jacobsantos6 years ago

There are too many open tickets for PHP notices, but thanks. It was getting annoying seeing all of the notices. I think also the problem is that some notices only appear when the cron runs or you do an action.

comment:3 jacobsantos6 years ago

Need to close the other one as fixed.

comment:4 DD326 years ago

I realise there was another ticket, I assumed it'd have been closed (and hoped that the changes in it have been commited...)

See other currently open tickets #7411, #6669, #5752(uncommited patch, 5mths)

comment:5 DD326 years ago

Theres also complains over COOKIEHASH not being defined when the database upgrade kicks in too.

comment:6 santosj6 years ago

You are right, the ticket I made was committed to 2.6 and closed. I would like to get rid of the COOKIEHASH notice also.

comment:7 santosj6 years ago

From #5752:

These appear to not be fixed from the patch in the ticket, from #6669 patch.

  • wp-admin/import/blogger.php
  • wp-admin/includes/user.php
  • wp-content/themes/default/header.php
  • wp-includes/taxonomy.php

I'm going to check your current patch to see if you fix these issues. I think that eventually, the notices will be found again, but it is nice to have a record of all the current patches of where notices have been found by people. Which is why I kept the #6669 ticket. There are currently only two tickets (that I know of) that exist on this issue.

#6669 isn't completed, but if you want to close that to keep this one open, then that might make more sense. However, it will be a good idea to go over the duplicated tickets referenced in #6669 to make sure something wasn't forgotten.

DD326 years ago

notice in generate_tag_cloud() & attempt at getting rid of the cookiehash message

comment:8 DD326 years ago

attachment 7509.2.diff added.
notice in generate_tag_cloud() & attempt at getting rid of the cookiehash message

The cookiehash can probably be handled better, That's mainly me just wanting the message gone so i could focus on the other errors.

The tag cloud on the other hand, checks to see if the ID is set, else sets it to the key... on the other hand, the input data should be correct..

comment:9 ryan6 years ago

(In [8646]) Notice fixes. Props DD32. see #7509

comment:10 ryan6 years ago

Maybe we can get rid of the WP_INSTALLING conditional around COOKIEHASH. I don't think we really need it.

comment:11 ryan6 years ago

(In [8647]) Notice fix. see #7509

comment:12 mrmist6 years ago

In [8646] the change in wp-admin/includes/theme.php to

$page_templates[trim( $name )] = theme_basename( $template );

Seems to break the "Write new Page" screen. In the svn version I just checked I get

Fatal error: Call to undefined function theme_basename() in /home/www/svn/trunk/wp-admin/includes/theme.php on line 47

From page-new.php. Dunno if the svn is meant to be stable or not so whether that's a valid bug in it's own right I am not sure. :)

comment:13 DD326 years ago

$page_templates[trim( $name )] = theme_basename( $template );

Ah yes, Can someone revert trunk/wp-admin/includes/theme.php please? Thats a line from a patch for something else.. I must've missed it on the file list when creating the patch.

comment:14 westi6 years ago

(In [8657]) Revert accidental change. See #7509.

comment:15 ryan6 years ago

(In [8732]) Notice fixes. see #7509

comment:16 jacobsantos6 years ago

New patch fixes more notices. Going through and correcting more notices. There aren't any real major changes to the code, so the patch should be ready to go.

jacobsantos6 years ago

Fix wp-admin notices based off of r8877

comment:17 jacobsantos6 years ago

New patch corrects every notice I could find. Doesn't seem to be any more, but probably not looking hard enough.

DD326 years ago

comment:18 DD326 years ago

attachment 7509.3.diff added.

patch fixes a notice error in feeds, and an undefined variable in a feed-related function.

comment:19 jacobsantos6 years ago

Can these two patches be committed? It is very annoying when WP_DEBUG is turned on.

comment:20 jacobsantos6 years ago

  • Keywords commit added

Can the last two patches go in? Probably won't fix the ticket until the administration panel changes stop and the notices can be checked again, however, it would be nice to have these two patches in to ensure that new notices are corrected instead of releasing patches that already fixed the notices.

comment:21 westi6 years ago

(In [8944]) Notice fixes see #7509 props DD32 and jacobsantos.

comment:22 westi6 years ago

All in bar the widget fixes as they didn't seem to apply no more.

comment:23 westi6 years ago

(In [8945]) Notice fixes for the widgets admin page see #7509.

comment:24 mrmist6 years ago

  • Summary changed from Fix up some Notice's in admin to Fix up some Notices in admin

jacobsantos6 years ago

More corrections that didn't make the last cut. based off of r8976

DD326 years ago

comment:25 DD326 years ago

attachment 7509.4.diff added.

Fixes no-styling-on-install which stems from a PHP warning in the attribute about an indefined variable.

That file is allways called directly, So i dont see a need for a full absolute path, relative should do it.

comment:26 ryan6 years ago

(In [9146]) Use relative path for install css. Props DD32. see #7509

DD326 years ago

comment:27 DD326 years ago

attachment 7509.5.diff added.

[9000] doesnt appear to have fixed the problem completely.

I was having an issue where $wp_filter[$tag] was set, however [$tag][$priority] wasnt, which would cause a error to rise.

Note: Also needs to be applied to BackPress

DD326 years ago

includes 7509.5.diff

comment:28 DD326 years ago

attachment 7509.6.diff added.

  • includes changes from .5.diff
  • Fixes a notice level error on times when wp_uploads_dir() returns a error condition(ie. It couldnt create a folder for this month)
  • Fixes 2 notices on canonical errors, It assumes path? is set, which it isnt allways for http://domain.com site urls.

comment:29 westi6 years ago

(In [9177]) Notice fixes. See #7509 props DD32.

DD325 years ago

DD325 years ago

comment:30 DD325 years ago

7509.7.diff (1.6 kB) - added by DD32 on 10/20/08 01:26:14.

Ignore that patch

7509.diff (1.9 kB) - added by DD32 on 10/20/08 01:33:40.

oops, I checked the replace option.. Patch includes a notice on export, and a notice in the unique filename code (When using it as such wp_unique_filename('filename_no_extension')

comment:31 ryan5 years ago

(In [9270]) Notice fixes from DD32. see #7509

DD325 years ago

ensure ->response is set on update_plugins option to prevent menu notice.

comment:32 DD325 years ago

attachment 7509.8.diff added.

  • ensure ->response is set on update_plugins option to prevent menu notice when no plugins have a avaialble update.

comment:33 markjaquith5 years ago

(In [9323]) Fix a PHP Notice. props DD32. see #7509

comment:34 markjaquith5 years ago

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

Closing 'er up for 2.7

DD325 years ago

comment:35 DD325 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Closing 'er up for 2.7

Sorry! Its likely there'll be a few more before 2.7 ships :)

attachment 7509.9.diff added.

  • Fixes 2 notices which spawn on Editing posts page, $temp_id is not initialised, which causes a notice, and $post_id is passed instead of $post_ID

comment:36 ryan5 years ago

(In [9332]) Warning fixes from DD32. see #7509

filosofo5 years ago

Viper007Bond5 years ago

Superceeds filosofo's patch (i.e. includes their changes)

comment:37 ryan5 years ago

(In [9506]) Notice fixes from filosofo and Viper007Bond. see #7509

Viper007Bond5 years ago

comment:38 ryan5 years ago

(In [9515]) Notice fixes from Viper007Bond. see #7509

DD325 years ago

comment:39 DD325 years ago

attachment 7509.10.diff added.

  • (Not a warning) Use strlen() when mb_strlen() is not available for title length (Unlikely those with double-byte characters will be using PHP without mb, so limited impact)
  • Correct a seemingly bad copy of the Edit-tags page to add-tags
  • Fix a defaults warning on the dashboard options on first page load
  • Fix warnings related to Autosaves when no except/content is available
  • Fix autosave when creating a new post/page (I believe this may fix the root cause of #6728)
  • Fix warning in Screen options for when a page doesnt contain any table columns
  • Fix a warning on install when Title/email/show_public is not provided
  • Fix warnings on page/post publish related to undefined array index's
  • Fix a warning on the pinging when a link is not parseable (ie. http://)
  • Pass the User-ID of the current user along with Autosaves, fixes a warning related to the publish code expecting a user_id or custom post_author to be passed
  • Set a default list of tags for publish (Empty tag list)
  • Ensure that the Currently-queried-page is a valid page before attempting to use it in query
  • Fix warnings in term sanitization on Add-tag/category pages related to non-set index's
  • Extract the Widget inputs in the options page, Fixes undefined variable notices
  • define COOKIEHASH on all pages, Removes warnings on installing - As per discussion near the start of this ticket

(Phew, Sorry for the big patch :P)

comment:40 DD325 years ago

Also just a quick note:

  • There's more notices on attempting to add new Links / edding links

comment:41 ryan5 years ago

(In [9596]) Notice fixes from DD32. see #7509

DD325 years ago

comment:42 DD325 years ago

attachment 7509.11.diff added.

  • Registration unset vars
  • Warning on retrieve new password page
  • isset() in RSS (Its inside the magpie class i believe, I dont think thats being maintained anymore upstream?)
  • isset() in some post functions, and some trailing whitespace cleanup of tag inputs

DD325 years ago

DD325 years ago

comment:43 DD325 years ago

attachment 7509.13.diff added.

  • Replaces both 11 and 12
  • Includes Notices of undefined post_password on add new page/post pages

comment:44 ryan5 years ago

Can we init post_password in get_default_post_to_edit() and avoid those isset checks?

comment:45 DD325 years ago

Can we init post_password in get_default_post_to_edit() and avoid those isset checks?

That would indeed be a better location for it, I wasnt aware of that.

An even bigger patch is aboutto be submitted with a lot of other stuff included too..

Things to note of 7509.14.diff

  • Replaces 11+
  • Includes user-new fixes
    • Remember passed details on failing to add user, default to blank strings
    • Redirect to correct search page afterwards
  • Dashboard: Right now
    • The dashboard basically assumes theres posts posted, In the case where there are no posts, things break, That fix in the patch isnt the best, Its mainly as a reminder to whoever finishes all the TODO's (The other code branches probably suffer the same thing)
  • a few more random notice fixes

DD325 years ago

comment:46 ryan5 years ago

(In [9699]) Notice fixes from DD32. see #7509

DD325 years ago

DD325 years ago

comment:47 DD325 years ago

attachment 7509.15.2.diff added.

  • Ignore 7509.15.diff
  • Media notice of undefined variable / variable not used
  • RSS snoopy compat for the HTTP API, some parts of the code use ->respose_code instead of ->status
  • Query: Search term checking with empty() & unencapsulated string

comment:48 ryan5 years ago

(In [9714]) Notice fixes. Props DD32. see #7509

comment:49 ryan5 years ago

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

comment:50 hakre4 years ago

Related: #14855

Note: See TracTickets for help on using tickets.