This is not a big one, but I couldn't figure it out myself.
I have a Multimedia asset field in a content type and you can upload unlimited files. After you select/upload one file and click the button "Add another item", then click the "select media" again, you get an iframe with admin toolbar which looks abit funny. This did not happen in beta3.
I've found out the source url (widget.src/media.browserUrl) somehow douplicates when you add another item: from "/media/browser?render=media-popup..." to "/media/browser?render=media-popup,/media/browser?render=media-popup..." and 3 times for 3 files etc.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SanderJP’s picture

Status: Active » Needs review

Well I dived in it myself and found that each time you click "Add another item" it adds more entry's in the element array (in media.browser.inc at function 'media_attach_browser_js()' line 301). The variable browserUrl should be the same for each item so doesn't need to be added for each item seperatly.
The problem is in the function 'Drupal.media.popups.mediaBrowser.getDefaults' in media.popups.js (line 107) where it loads the browserUrl. If there is one item, the variable contains '/media/browser?render=media-popup' but more items turns it into an array and the variable outputs '/media/browser?render=media-popup,/media/browser?render=media-popup'.

My solution is not the best way but as far as I've tested it, it works. In media.popups.js, at the function 'Drupal.media.popups.mediaBrowser.getDefaults' (line 107) i've added an if statement to check wether it's an array or not. If so, only take the first entry of the array:

var browserUrl = Drupal.settings.media.browserUrl;
if (browserUrl.constructor.toString().indexOf("Array") != -1) {
	browserUrl = Drupal.settings.media.browserUrl[0];
} else {
	browserUrl = Drupal.settings.media.browserUrl;
}

And at line 121, I changed:

src: Drupal.settings.media.browserUrl; // Src of the media browser (if you want to totally override it)
into:
src: browserUrl, // Src of the media browser (if you want to totally override it)

As I said for now it works, but I'm not sure this is the best way to solve it.
Feedback/comments/improvements are welcome :)

james.elliott’s picture

Status: Needs review » Needs work

I would think that the correct fix would be to solve the bug that is turning the url into an array in the first place. Your suggestion fixes the symptom but not the issue.

This will also be irrelevant once #1139514: Overhaul the media browser code to not use an iframe, and be more understandable, maintainable, and extendable makes its way into the main branch.

SanderJP’s picture

yea, this is just a workaround, will keep an eye on the fat-trimming :)

katbailey’s picture

Adding sanderjp's work-around as a patch seeing as there's not much point in finding a better solution in parallel with the fat-trimming work that's happening, but there's no telling when that one will make it in and I need this in my make file right now ;-)

langworthy’s picture

Version: 7.x-1.0-beta4 » 7.x-1.x-dev
FileSize
1.39 KB

re-roll of #4 against HEAD

langworthy’s picture

#5 is not against HEAD. I'm not sure what I was thinking

webflo’s picture

thx for the work a round in #5.

madeby’s picture

This is still an error in RC1 - I really needs to be addressed for a final release.

gaele’s picture

Status: Needs work » Reviewed & tested by the community

#5 is working fine for me too.

Edit: #4 is working fine for me too.

The last submitted patch, popup-browserurl-1144422-5.patch, failed testing.

gaele’s picture

Status: Reviewed & tested by the community » Needs review

#4: 1144422-popup-browserurl.patch queued for re-testing.

Dave Reid’s picture

I'd rather fix the bug here as well rather than the symptom.

Chris Matthews’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)

Closing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team