WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#22553 closed defect (bug) (fixed)

PHP notices when inserting images

Reported by: miqrogroove Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: major Version: 3.5
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

Using WP 3.5-RC1, Chrome 23, and WinXP, I found that I could upload two images from QuickPress on the first try, but when I clicked the "Insert into post" button, the upload screen went away and the QuickPress textarea remained totally blank. This problem is repeatable.

Attachments (3)

22553.patch (1.1 KB) - added by azaozz 3 years ago.
22553.diff (2.9 KB) - added by nacin 3 years ago.
Includes fix for #22544.
22553.2.diff (2.7 KB) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 @miqrogroove3 years ago

  • Summary changed from Can't Insert Images in QuickPress to Can't Insert Images

Correction, I can't insert images at all! I just switched to the post editor expecting that to solve the problem, but I still have a totally blank post after clicking Insert.

comment:2 @miqrogroove3 years ago

  • Severity changed from major to blocker

Same problem in Firefox 16.
Same problem in Opera 12.
Same problem in IE8.
Confirmed with all plugins deactivated.

The only browser I have that can insert posts now is Safari on my iPad.

This needs to block RC2.

comment:3 @johnbillion3 years ago

Confirmed. Clicking 'Insert into post' does nothing. Tested at r22829.

comment:4 @SergeyBiryukov3 years ago

Could not reproduce yet neither in 3.5-RC1 nor in trunk. My steps:

  1. Open post editor (or QuickPress).
  2. Click "Add Media".
  3. Upload an image, or select an existing one from Media Library tab.
  4. Click "Insert into post".
  5. The image appears in the post.

Tested in Firefox 17, Chrome 23, IE 8, Opera 12.

comment:5 @miqrogroove3 years ago

I'm available for testing if you have any ideas about how to debug this.

comment:6 @ocean903 years ago

  • Keywords reporter-feedback added
  • Severity changed from blocker to normal

It's not a blocker until you can provide detailed steps to reproduce the issue. Currently I can't reproduce it.

comment:7 @miqrogroove3 years ago

  • Severity changed from normal to blocker

Okay ocean90, that doesn't help at all. What would you like me to do?

comment:8 @miqrogroove3 years ago

I have partial debugging information.

In media-views.js

controller.state().trigger( 'insert', selection ).reset();

This line of code does not work. On my iPad it leads into the insert function in media-editor.js. On Windows, it doesn't seem to do anything at all.

comment:9 @miqrogroove3 years ago

I'm brainstorming with azaozz on IRC. I've determined that in media-editor.js, this line is never executing:

wp.media.editor.insert( resp );

azaozz thinks it has to do with PHP errors being generated during ajax. I've got this showing up repeatedly in logs:

[23-Nov-2012 20:08:22] PHP Notice:  Undefined index:  url in /wp-admin/includes/ajax-actions.php on line 1945
[23-Nov-2012 20:08:22] PHP Notice:  Undefined variable: rel in /wp-admin/includes/ajax-actions.php on line 1951

comment:10 @miqrogroove3 years ago

  • Summary changed from Can't Insert Images to Can't Insert Images in Debug Mode

Confirmed, images can be inserted normally with DEBUG turned off.

@azaozz3 years ago

comment:11 @azaozz3 years ago

22553.patch fixes couple of PHP notices that show at the top of the ajax response and break it.

Version 0, edited 3 years ago by azaozz (next)

comment:12 @nacin3 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.5
  • Severity changed from blocker to major
  • Summary changed from Can't Insert Images in Debug Mode to PHP notices when inserting images

comment:13 @miqrogroove3 years ago

  • Keywords has-patch added

IRC discussion has wrapped up. Patch tested and working. +1 commit

@nacin3 years ago

Includes fix for #22544.

comment:14 @nacin3 years ago

Looking at this, I think we should go back to the old field names used by media_upload_form_handler() — align, image_alt, image-size, post_excerpt, post_title. Here's a patch that does that while fixing both this ticket and #22544.

Last edited 3 years ago by nacin (previous) (diff)

comment:15 @nacin3 years ago

  • Keywords commit added

Needs review and commit.

@nacin3 years ago

comment:16 @nacin3 years ago

Refreshed 22553.diff. Working well for me.

comment:17 @miqrogroove3 years ago

Re-testing on 3.5-RC1-22844

Without any patch I get

[26-Nov-2012 17:23:45] PHP Notice:  Undefined index:  post_id in /wp-admin/includes/ajax-actions.php on line 1935
[26-Nov-2012 17:23:45] PHP Notice:  Undefined index:  url in /wp-admin/includes/ajax-actions.php on line 1950
[26-Nov-2012 17:23:45] PHP Notice:  Undefined variable: rel in /wp-admin/includes/ajax-actions.php on line 1956

With patch 22553.2.diff​ I get

[26-Nov-2012 17:32:24] PHP Notice:  Undefined index:  post_id in /wp-admin/includes/ajax-actions.php on line 1935

comment:19 @ryan3 years ago

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

In 22847:

In wp.media.editor:send, revert back to the old field names used by media_upload_form_handler() for easier back compat.
Fix notices in wp_ajax_send_attachment_to_editor().

Props nacin, azaozz, miqrogroove
fixes #22553

Note: See TracTickets for help on using tickets.