With the Upload, Web, and Library tabs enabled, after uploading an umage using the upload tab, the user gets automatically switched to the Library tab once the ajax file upload progress bar has completed. This is confusing as the uer has no idea what has happened of if there image has been uploaded or not (it doesn't show in the Library tab). Switching back to the Upload tab shows that the image has been uploaded, and clicking save works as expected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrfelton’s picture

Title: After uploading image using upload tab, user gets switched to Library tab » Ajax callbacks in the media overlay cause the active tab to switch tab unexpectedly

A similar thing happens when using the pagers at the bottom of the Library tab. As soon as the ajax callback has completed and the Library content is updated to show content from the next page, the user is automatically switched to the Web tab. Clicking back to the Library tab shows the updated Library view.

So, it seems like it may be a problem with ajax callbacks in general, causing the active tab to switch unexpectedly.

mrfelton’s picture

mrfelton’s picture

clemens.tolboom’s picture

clemens.tolboom’s picture

In media_youtube: #1551376-7: YouTube Tab: When apply searching, next or previous button it does not retain tab when loading result @Aaron added a solution for the tab provider.

In #1551376-8: YouTube Tab: When apply searching, next or previous button it does not retain tab when loading result I added comment for the 'refresh' flow copied here as Media should keep the tab state.

Media must manage the Tab state like this:

(http://stackoverflow.com/questions/4299435/remember-which-tab-was-active...)

 # javascript
$(".tabs").tabs({
    select: function(event, ui) {                   
       window.location.hash = ui.tab.hash;
    },
});

I'm not sure we need setter and getter as we have set the #fragment as above. Is that #fragment kept when submitting through one of the tab forms? I haven't found any jQuery UI Tab related javascript.

(http://api.jqueryui.com/tabs/#option-active)

 #javascript
// getter
var collapsible = $( ".selector" ).tabs( "option", "collapsible" );
// setter
$( ".selector" ).tabs( "option", "collapsible", true );
clemens.tolboom’s picture

Status: Active » Needs review
FileSize
1.72 KB

Attached patch solve this for a few tabs which triggers media_form_alter.

It works for media_youtube Search button. Not for #1551376: YouTube Tab: When apply searching, next or previous button it does not retain tab when loading result

It fails for Library or My files as the hook_form_alter is not involved for those page.

It needs more work but I need a review :)

clemens.tolboom’s picture

Component: Code » Media Browser

Patch from #6 works nicely.

Tested against:
- Upload (it is the first tab after all)
- YouTube
- Web
- Remote URL
- Library
- My files

gmclelland’s picture

Just tested the patch in #6 and everything seems to work correctly. Great job!

slashrsm’s picture

Hm... I'm unable to reproduce this issue on latest -dev. Can anyone help me? Do I have to do anything special to see this error?

gmclelland’s picture

@slashrsm - You can see the problem when you have the media_youtube module installed. On the youtube tab in the media browser try to search for something. The mediabrowser iframe will reload, but the tab will no longer be on the youtube tab it will be on the upload tab.

This patch solves the problem by keeping the user on the youtube tab after searching.

Hope that helps.

slashrsm’s picture

Status: Needs review » Needs work

Great job!

As already mentioned in #6 it fixes media_youtube, but it does not fix views pager if not using AJAX (Library, My files, ...). I'm wondering if there exists any other, more general solution....

bneil’s picture

Here is another tab-related issue #1934226: Hide tabs when plugins are on step > 1.

clemens.tolboom’s picture

Assigned: Unassigned » clemens.tolboom

Tnx for testing.

Regarding non ajax views: @slashrsm you are right this should be more general. Maybe we could intercept at the plugin level? I could use some hints.

@bneil thanks for the XREF.

clemens.tolboom’s picture

Issue tags: +ux

Tagging UX

tobiasb’s picture

FileSize
1.72 KB

This iss the patch for the current "2.0-unstable7" Version. Useful for drush make or so.

clemens.tolboom’s picture

Assigned: clemens.tolboom » Unassigned
ParisLiakos’s picture

we could use a cookie and be done with it..it would work on all occasions

timbrandin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, media-1802026.15.patch, failed testing.

timbrandin’s picture

Version: 7.x-2.x-dev » 7.x-2.0-unstable7
Status: Needs work » Needs review

Testing on unstable7 instead.

timbrandin’s picture

#15: media-1802026.15.patch queued for re-testing.

timbrandin’s picture

Here's a similar but improved solution, that stores the currently active tab in the Drupal.media var.

timbrandin’s picture

Added so that when the active tab is selected the resizing works as expected.

crazybutable’s picture

While investigating this patch, before I applied it, I tested the original issue and could not reproduce the bug with the library pagers.

jlk4p’s picture

Installing the first, media-unexpected-tabs-1802026-6.patch, worked. But it did not resolve the problem. Installing any of the other patches after that seemed to have an issue with the existing code. I went back and downloaded the module (7.x-2.0-unstable7) again and reinstalled so I can try this again. Then I installed just the last patch, media-browser-ajax-active-tab-issue-1802026-23, without an error. But it doesn't appear to return to the same tab when submitted.

Do I need to apply all the patches in this thread for the issue to get resolved?

KarlKedrovsky’s picture

Status: Needs review » Reviewed & tested by the community

Tested patch on #23 using the current media_dev distribution and it worked as expected.

Dave Reid’s picture

Version: 7.x-2.0-unstable7 » 7.x-1.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed #23 to 7.x-2.x: http://drupalcode.org/project/media.git/commit/83d93ab. Thanks everyone!

We should probably look if this needs to be backported to 7.x-1.x as well.

torotil’s picture

Status: Patch (to be ported) » Needs work

I just tried this with media 2.0-alpha1 and media_youtube-2.0-rc3. For me the patch doesn't work at all. Watching the variable Drupal.media.browser.activeTab revealed that it is always set to 0 when loading the form - regardless of which tab I select.

torotil’s picture

It's the non-ajax part (see #6 or #15) that's still unsolved.

ParisLiakos’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev
Status: Reviewed & tested by the community » Needs work

for the non-ajax part a $.cookie would be needed to be set
this is something i have used for a client project in an attach method:

     $('#media-browser-page form', context).submit(function () {
        $.cookie('mediaBrowserActiveTab', $('li.ui-tabs-selected a').data('tabid'));
      });
      $('#media-browser-page form input[type="submit"]', context).mousedown(function () {
        $.cookie('mediaBrowserActiveTab', $('li.ui-tabs-selected a').data('tabid'));
      });
      // Use jquery cookie to guess the default tab.
      if ($.cookie('mediaBrowserActiveTab')) {
        $('li a[data-tabid="' + $.cookie('mediaBrowserActiveTab') + '"]', context).click();
      }
torotil’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Needs work » Needs review
FileSize
2.8 KB

Here is a patch that keeps the page using the window.location.hash. It works with AJAX as well as without.

+It makes the selectErrorTab function obsolete.

torotil’s picture

Here is the same patch for 7.x-1.x.

torotil’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev
bneil’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
aaron’s picture

Status: Needs review » Reviewed & tested by the community

This works for me.

jlk4p’s picture

The patch in #31 above fixed the issue with Media 7.x-2.0-alpha2 for me.

hefox’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

Patch no longer applies to 2.x-dev

torotil’s picture

Here's a re-roll of the patch. Now also works with jQuery-UI >= 1.10.

Let's see how many RTBCs and re-rolls this needs before it gets in.

torotil’s picture

Status: Needs work » Needs review
torotil’s picture

Status: Needs review » Needs work

The last submitted patch, 40: media_browser-keep-active-tab-1802026-39.patch, failed testing.

torotil’s picture

Status: Needs work » Needs review
FileSize
3.43 KB
SiliconValet’s picture

The latest patch (#42) doesn't work in the latest 7.x-2.x-dev release. That is, after applying the patch, when I get the popup and select the library, clicking the next button (pager item) sets the upload tab (for me anyway - seems it could be any tab for others) as the active tab.

In short, no difference before or after the patch.

UPDATE: In playing with it, it seems it's not returning the index and resorts to '0'. For my personal use, I've set it to the index I need. I found this near line 49 (after patch #42) here: "Drupal.media.browser.tabFromHash = function () {"

hefox’s picture

Status: Needs review » Needs work

I got a patch working for the settings of jquery update I'm using and the alpah3, which may illustrate why some people are having issues with this patch

window.location.hash doesn't work cross browser; I was completely unable to get it to work in chrome, was always empty.

ui.tab is not set in some jquery ui versions, it's like nextTab, but might as well use a selector if possible.

I changed it to store the tab info in an attr, after trying various things including jquery.data and having none of them persist. I think that is a bad idea but works.

hefox’s picture

SocialNicheGuru’s picture

the patch no longer applies to dev media.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
3.48 KB

Here's a reroll which applies to the latest 7.x-2.x code. Seems to work (based on light testing).

SocialNicheGuru’s picture

this patch works well. thanks!

alex.skrypnyk’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #47 work really well. It also handles jQuery <1.9 and >=1.9 and jQuery UI < 1.8 and >= 1.8

The last submitted patch, 45: 1802026-media-browser-keep-tab-43-alpha3-DO-NOT-TEST.patch, failed testing.

  • aaron committed b534d3e on 7.x-2.x
    Issue #1802026 by David_Rothstein, hefox, torotil, timbrandin, tobiasb,...
aaron’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.