Hi,
I've created the "Media Multiselect" module (http://drupal.org/sandbox/fangel/1652676), and got a report that the multiselection-feature broke on page 2 of the browser (#1815812: multiselect doesn't work after using pagination).
So I went to investigate, and it wasn't just multiselection that broke on page 2 - all Media Browser parameters (multiselect, types, etc) gets reset on pagination. This is because the Media Browser parameters is only ever detected in the menu-callback media_browser()
. On AJAX-requests, this menu-callback is never called, so the requests are never parsed and stored, and hence it will send along an empty list of parameters.
So I've created a patch, that moves the parsing of parameters out of the menu-callback, and into media_set_browser_params
.
I've also done a slight alteration to the browser-js so it deselects any files selected on previous pages.
So please review the patch, so we can get the AJAX-pagination working properly with non-standard browser parameters.
Kind regards
Morten.
Comments
Comment #2
fangel CreditAttribution: fangel commentedRerolled patch.
Comment #3
fangel CreditAttribution: fangel commentedComment #4
ParisLiakos CreditAttribution: ParisLiakos commentedEverything looks good from code perspective..just this really small one.
is there a reason we add the processed class there and not in the end of the function, when it is actually processed?
Comment #5
fangel CreditAttribution: fangel commentedNo, that might make sense - I sometimes like to group the two lines together for ease of reading, but I guess it should belong at the bottom. I'll revise the patch later today, or tomorrow..
Comment #6
fangel CreditAttribution: fangel commentedOkay, patch updated - the addClass call is moved to the bottom now.
Comment #7
ParisLiakos CreditAttribution: ParisLiakos commentedThanks..I will check its functionality once more and then commit it
Comment #8
xcafebabe CreditAttribution: xcafebabe commentedPatch #6 works as expected! Kudos to @fangel
Comment #9
ParisLiakos CreditAttribution: ParisLiakos commentedjust a question..why we need this?
+ $params = media_set_browser_params($params);
inside the very same function?
I guess it is a copy-paste failure since $params parameter is removed now.
Also i am trying this with unstable7 and your latest code from your sandbox and it is not working..debugging further
Comment #10
fangel CreditAttribution: fangel commentedAhh, yes - correct. It is unneeded. I removed it.
I'm running git versions of Media, File Entity and my sandbox, fwiw.
Comment #11
ParisLiakos CreditAttribution: ParisLiakos commentedah ok..i was actually expecting it to remember the selections i made in page 1 when i was in page 2..
So if i select 2 items from page 1 and then 2 items from page 2 all 4 items are being added when i hit submit button.
But thats not the case and i guess a different issue..
thanks
Comment #12
ParisLiakos CreditAttribution: ParisLiakos commentedOk, commited it. thanks again
http://drupalcode.org/project/media.git/commit/7a2b3e6
Comment #13
fangel CreditAttribution: fangel commentedrootatwc: Yes, because there is currently no GUI to display items selected on the previous page, I made the patch so that it clears any selection when you go to a new page.
This makes it consistent with switching library tabs (if you select something in library and in my files, both selections are not used)
Thanks for committing..