Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 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 9 years ago.
Patch which removes the postID variable and uses wp.media.view.settings.post.id
33096.1.patch (988 bytes) - added by rhurling 9 years ago.
Refreshed patch, since the first didn't seemed to work
33096.2.patch (4.7 KB) - added by iseulde 9 years ago.

Download all attachments as: .zip

Change History (9)

@rhurling
9 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.


9 years ago

#2 @iseulde
9 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
9 years ago

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

@iseulde
9 years ago

#3 @iseulde
9 years ago

  • Keywords has-patch added; needs-patch removed

Fixes dependencies as well.

#4 @iseulde
9 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
9 years ago

  • Milestone changed from Future Release to 4.3

#6 @iseulde
9 years ago

In 33428:

JSHint for [33426]

Oops. :|

See #33096.

Note: See TracTickets for help on using tickets.