Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to reproduce:
* Use Media 7.x-2.11 + WYSIWYG + Media WYSIWYG
* Modify an image (click "Add Image" with image selected in WYSIWYG), media/%fid/format-form
will appear in modal iframe
* Click "Edit image"
* Edit image page will appear in popup, but with admin theme header/footer, admin menu inside iframe
Better would be to preserve the ?render=media-popup
in the Edit Image link, then we will not see breadcrumb / admin theme elements / admin menu in the edit image page at file/%fid/edit
Before:
Current behaviour on clicking "Edit Image":
Comments
Comment #2
xurizaemonPatch attached, here's the improved behaviour.
Screenshots show the modal content only, not the full page.
Comment #3
joseph.olstadhmm,
render is already listed as a safe option in media.module . These are cached and pulled from cache, maybe have a look at the way we're using cache_set and cache_get and the options.
There's a simpletest for this you can look at the code.
have a look at sa contrib which brought in cache_set and cache_get
https://drupal.org/node/2877316
Comment #4
xurizaemon@joseph.olstad, thanks for the quick reply and pointer to [#2877316], I will review that ... but I don't see how that being a "safe option" would preserve the render= parameter when generating that Edit Image button.
To be clear: this behaviour changes when navigating from "Embedding image.jpg" to "Edit image.jpg" in the modal.
Currently the link is generated in modules/media_wysiwyg/includes/media_wysiwyg.pages.inc@180 with:
l(t('Edit file'), 'file/'.$file->fid.'/edit', array('attributes' => array('class' => 'button', 'title' => t('Use for replace file or edit file fields.'))))
The patch above adds 'query' array entry to preserve the render param in button, so the linked form behaves as the current one.
Unless some filter is happening after that
l()
, or some JS is being applied? (Or, I'm misunderstanding your comment?)Let me know if this needs to be submitted for 8.x-x.x as well in order to be considered, happy to supply a matching patch for that branch.
Comment #5
joseph.olstadok, the patch looks good , although I haven't actually tested it but I did review it.
shouldn't cause any security issues as the render parameter is already whitelisted.
Comment #6
joseph.olstadactually, on second thought, would it be possible to try this patch with media 7.x-2.11?
For what reason are you still running version 2.2? is there a compatibility issue?
Comment #7
xurizaemonMy mistake, should have said 2.11 in the description. Patch is against 2.x-dev and tested against 2.11.
Comment #8
joseph.olstadComment #9
joseph.olstadon the radar for commit soon
Comment #12
joseph.olstadThanks @xurizaemon, keep 'em comming.