Unable to select files when two or more Views browser plugins are active

I have error when i click file in view library tab
mediaFile is undefined
http://localhost/sites/all/modules/media/js/media.js?m4s0k8 Line 45

Yep, I'm encountering this too as well. Putting on a fast-track to fix today.

So I'm thinking we have two possible solutions here:
1. Apply the temporary fix similar to #2.
2. Change the Media browser to request the JSON file object via AJAX when requested, rather than pre-load every single possible file in Drupal.settings.media.files.

Or, another alternative:
3. Place the JSON inside a HTML5 data attribute in the .media-item row itself. This would be automatically fetchable with http://api.jquery.com/data/#data-html5

Why insist on making it super complicated? Why insist on calling the solution proposed in #2 'a temporary fix'?

Since introducing the 'My files' tab, this bug will always be triggered because now two views-backed displays are shipped by default. Hence the re-key'ing will always happen, so I don't see why you shouldn't just accept that and use it as an array. No need trying to re-cast it to an hash-map style object.

Why not just ensure that Drupal.settings.media.files is always an array and then just iterate through the files-array to find the object with it's fid-property equalling the data-fid-property of the selected image.
That is easily doable - simply add an array_values in the drupal_add_js line in media.views.js:88 and by using a simple Array.filter to reduce the Javascript-array to only contain the right file.
It solve the issue by changing two lines!

I fail to see what added benefits solution #2 or #3 bring? They only bring a ton more complexity - especially solution #2, ajaxifying the data-gathering. So why would you ever go that way, and not just use this very permanent "temporary" fix.

Well holy hell, I'm sorry for ever trying to think an issue through with the possible solutions to ensure that we not only fix the issue, but we fix any underlying issues or architecture behind it. I was going to come back today and update that the "temporary" fix is most definitely the right way to go for now, but I guess you've already decided.

My thinking is that I'm wondering if adding the JSON for each file in Drupal.settings is actually not a great thing to be doing, and using data attributes for the JSON would be better. And for reference, I was able to implement #2 in about 2 minutes and it just works as well.

I'll get on this today with implementing #1.

@DaveReid: Take a deep breath, buddy, and go pet Rodney for a while.

@fangel: Thank you for finding a solution to the problem, we all appreciate your effort. While it is a solution, Dave wanted to delve deeper into the problem to find out what was really causing the problem and identify the best solution, and it seems that he's close to working it. out.

I wasn't trying to be hostile. I was just confused why the - imho - simple solution was first categorized as temporary.

As far as I know, the current solution of using Drupal.settings has worked great, then problems with drupal_add_js re-key'ing the array popped up. So it just seemed strange to want to go out and do a fairly big restructuring to something that - again to my knowledge - has worked splendidly so far, and only requires very few lines to fix.

But if you think there are architectural reasons for why something other that the current approach is needed, I believe it can only benefit from these reasons being discussed.
Sorry for my somewhat harsh tone, I just personally see no reason to replace something only slightly broken with something much more complicated and untested without good, known reasons.

Note that the JavaScript filter() function is not available in all browser, specifically IE versions lower than 9: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Arr...

For what it's worth, here's an updated patch that uses a for()-loop instead of filter.

I'm not sure why the array_values() change is necessary. Revised patch with changes only to media.views.js.

Merged #1417436: Submit and Cancel Buttons on Views Tab's don't do anything into this issue which has lots of helpful comments.

I had the array_values() there to ensure Drupal.settings.media.files was an array, not an object (hashmap-style).

If you don't have that, and only have one call to drupal_add_js, then Drupal.settings.media.files will have fid-as-keys, which results in it being encoded as a Javascript object (hashmap), and then the for-loop wont work, because it's trying to traverse over an object, and not an array.

I'm copying this from the duplicate because this is where we ended up,
http://drupal.org/node/1417436#comment-6055488

I have attached a patch which is an extension of #25.

This patch does two things.

1. Use the LI ID as the source of the media fid.

Reason: Previously a wrapper A was used to get this. It had an attribute that stored the data, but because of the styling / structure, this wrapper would not actually wrap the media-item DIV. I believe this was because there were A's inside A's and this caused issues. All of the items are inside LI's, so I modified the jQuery traversing script to find this and take the number off the end of the ID attribute. The structure is also different between PDF media files and Image ones.

2. Fix the selection of the media item (this is pulled from #25.

Reason: #25 fixes an issue where the FID was used to look up and attach the media object, however (as was described previously with the associative array vs object), the IDs were stored as values, not as keys. This patch goes over each item in the array and checks the value.

This patch is sort of hacky, but so is everything here. I'm not sure what the underlying issues are, but this seems to fix the issue for me.

I think this is still relevant because the previous patches still look like they are using the "parent" traversal which breaks in the situations explained above.

I think combining #15 and #16 makes sense to me to ensure the patch works for single and double views. Fangle's comments make sense. It was also proposed in the other thread to use a string replace function instead of substr(), which I think makes sense to make it more readable, although both will fail if the ID naming changes.

I feel that these fixes should be rolled out since the views are currently broken, but that the underlying structure needs to be eventually fixed. The broken A inside A is one of the issues here and even with the fix in #16, you can't click on the thumbnail to select the media.

This one combines #13, suggestions from #15, #16 and I think I improved the fid finding code. Give it a shot.

redndahead,

This traversal will not work for my install because some of the media files have no wrapper anchor.

Is anyone else having this issue? Have you tried with PDF / Image / Video media files?

@pandaeskimo: Hrm, I can't confirm not having a wraper anchor. Did you change the 'Preview' view mode display for any file types? Or are you using the File styles module at all?

We have separate views for images, videos, files and docs. It's almost an exact clone of the default media browser view. They all have the a tags.

Do you have a data-fid attribute? Maybe we can just search for that.

Ok, we still had file_styles enabled and it is causing issues. It messes up the wrapper anchors.

I think there was some documentation about disabling it, but we migrated from Media 1.x which was the cause of the issue. Is there some way to notify users to disable that old module if it hasn't already been done. If it has, sorry.

I think we should be good with #18!

Now I have to see if any styles broke without File Styles.

We can remove the explicit a in the search. So it becomes

var fid = $(this).closest('[data-fid]').attr('data-fid');

I think the only issue with that is it may be a bit slower. Not that it really makes much of a difference in this case. That would let it be any tag with the data-fid attribute.

Would that have fixed your issue pandaeskimo?

(Note, I'm using jQuery Update to get jQuery 1.7, so not sure if this is available in the bundled jQuery) Why not use .data('fid') instead of using .attr('data-fid')?

using .data('fid') is available since jQuery 1.4.3 and Drupal 7 uses 1.4.4 by default so that's clear for us to use as far as I know.

No, it wouldn't have fixed the problem since there was no wrapper. I think we can ignore my issue as a result of having File Styles which should be disabled when switching from Media 1.x to 2.x. We are good to go with the previous patches I think!

Thanks.

Ok here is the updated patch.

Needed to update the patch. It seems that data returns numeric value and the settings is a string. Fixed it by not being so strict on the type in the comparison.

FWIW, I ran into a problem not having the array_values call there, so it's very much so needed. After AJAX-reloading one of the views in the tabs, it would send back the new files as a hash-map style object literal, which the AJAX-settings-merger didn't want deal with (merging a object into an array), so it just Drupal.settings.media.files with the new object, instead of appending to the array.
This broke both the foreach-code to identify the selected file, and all other tabs as their file-infomation had been thrown away in the bad merge.

With array_values there, it merges the new settings with the old, and everything keeps working.

Since the current patch has array_values there can you verify that it works for you?

It looks good, but I wont be back at work till Monday, so I'll test the final patch out against our production systems then and report back.

Yes, the patch in #29 works like a charm.

Ok I've committed #29 to 7.x-2.x with a couple of small changes after doing extensive testing with both one or two Views media browser plugin tabs enabled. Thanks everyone for your help on this issue.
http://drupalcode.org/project/media.git/commit/f0bfc42

I'm still seeing this error after upgrading from 7.x-2.0-unstable5 to 7.x-2.0-unstable6 (which contains the patch in #29). Cache has been cleared and I also tried disabling the "My Files" tab to see if it would work with only the "View Library" view enabled. Would love any insight on possible other conflicts. Happy to provide more configuration information -- let me know what would be helpful.

@mrosas are you using media_youtube? There was a bug in in recently that replicates the same issue described here.
#1627404: View Library submit button broken when media youtube is installed

if you are using media_youtube update to the lastest dev version.

-