Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#33096 closed defect (bug) (fixed)

Don't use hidden #post_ID input field to get mce-view embed postID

Reported by: rhurling's profile rhurling Owned by: iseulde's profile iseulde
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.9
Component: TinyMCE Keywords: has-patch
Focuses: javascript Cc:

Description

The mce-view embeds use postID = $( '#post_ID' ).val() || 0 to get the ID of the current post (see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/mce-view.js#L731).
Everything else I could find used wp.media.view.settings.post.id.

I think it would be better to either use wp.media.view.settings.post.id in the callbacks that use the current postID variable or switch to something that is localized to the script (brought up by @iseulde; https://wordpress.slack.com/archives/core-editor/p1437654722000160).

I will add a patch which uses wp.media.view.settings.post.id, because I'm not sure what else I would have to change to use a localize the script.

Attachments (3)

33096.patch (1.0 KB) - added by rhurling 10 years ago.
Patch which removes the postID variable and uses wp.media.view.settings.post.id
33096.1.patch (988 bytes) - added by rhurling 10 years ago.
Refreshed patch, since the first didn't seemed to work
33096.2.patch (4.7 KB) - added by iseulde 10 years ago.

Download all attachments as: .zip

Change History (9)

@rhurling
10 years ago

Patch which removes the postID variable and uses wp.media.view.settings.post.id

This ticket was mentioned in Slack in #core-editor by rhurling. View the logs.


10 years ago

#2 @iseulde
10 years ago

  • Component changed from Editor to TinyMCE
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 4.2.2 to 3.9

There's also wp.media.model.settings.post.id. We can't use any of these because the script does not depend on media-*.js. It needs its own setting.

It looks like your patch is reversed or something.

@rhurling
10 years ago

Refreshed patch, since the first didn't seemed to work

@iseulde
10 years ago

#3 @iseulde
10 years ago

  • Keywords has-patch added; needs-patch removed

Fixes dependencies as well.

#4 @iseulde
10 years ago

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

In 33426:

TinyMCE: views: use media setting to get post ID

Also fix dependency declarations and confusing variable names.

Props rhurling.
Fixes #33096.

#5 @SergeyBiryukov
10 years ago

  • Milestone changed from Future Release to 4.3

#6 @iseulde
10 years ago

In 33428:

JSHint for [33426]

Oops. :|

See #33096.

Note: See TracTickets for help on using tickets.