WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 7 months ago

#22553 closed defect (bug) (fixed)

PHP notices when inserting images

Reported by: miqrogroove Owned by: ryan
Priority: normal Milestone: 3.5
Component: Media Version: 3.5
Severity: major Keywords: has-patch commit
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 7 months ago.
22553.diff (2.9 KB) - added by nacin 7 months ago.
Includes fix for #22544.
22553.2.diff (2.7 KB) - added by nacin 7 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 miqrogroove7 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 miqrogroove7 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 johnbillion7 months ago

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

comment:4 SergeyBiryukov7 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 miqrogroove7 months ago

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

comment:6 ocean907 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 miqrogroove7 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 miqrogroove7 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 miqrogroove7 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 miqrogroove7 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.

azaozz7 months ago

comment:11 azaozz7 months ago

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

Last edited 7 months ago by azaozz (previous) (diff)

comment:12 nacin7 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 miqrogroove7 months ago

  • Keywords has-patch added

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

nacin7 months ago

Includes fix for #22544.

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

comment:15 nacin7 months ago

  • Keywords commit added

Needs review and commit.

nacin7 months ago

comment:16 nacin7 months ago

Refreshed 22553.diff. Working well for me.

comment:17 miqrogroove7 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 ryan7 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.