WordPress.org

Make WordPress Core

#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 17 months ago.
22553.diff (2.9 KB) - added by nacin 17 months ago.
Includes fix for #22544.
22553.2.diff (2.7 KB) - added by nacin 17 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 miqrogroove17 months 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 miqrogroove17 months 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 johnbillion17 months ago

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

comment:4 SergeyBiryukov17 months 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 miqrogroove17 months ago

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

comment:6 ocean9017 months 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 miqrogroove17 months ago

  • Severity changed from normal to blocker

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

comment:8 miqrogroove17 months 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 miqrogroove17 months 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 miqrogroove17 months 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.

azaozz17 months ago

comment:11 azaozz17 months ago

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

Version 0, edited 17 months ago by azaozz (next)

comment:12 nacin17 months 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 miqrogroove17 months ago

  • Keywords has-patch added

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

nacin17 months ago

Includes fix for #22544.

comment:14 nacin17 months 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 17 months ago by nacin (previous) (diff)

comment:15 nacin17 months ago

  • Keywords commit added

Needs review and commit.

nacin17 months ago

comment:16 nacin17 months ago

Refreshed 22553.diff. Working well for me.

comment:17 miqrogroove17 months 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 ryan17 months 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.