Problem/Motivation

I'm encountering an issue that I think is related to https://www.drupal.org/project/drupal/issues/3364088 on the 10.1.x-dev branch. Inserting media after navigating to the second (or more) page in the media modal redirects to /admin/content/media-widget/image with a bunch of query parameters appended rather than returning to the content add or edit page. I only encounter this issue when paging to the second (or more) page in the media modal. If I insert media before paging it works as expected.

Steps to reproduce

  1. Install Drupal 10.1.x-dev with standard profile
  2. Enable the Media and Media Library modules
  3. Add a media entity reference field to the article (or any) content type using the media library widget for the form display (in my testing I referenced image media)
  4. Create enough media entities so that the modal has more than one page of results
  5. Create an article with media. When inserting the media, page to the second page of results and click "Insert selected"

This redirects to:

/admin/content/media-widget/image?media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bimage%5D=image&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_hero_image&media_library_opener_parameters%5Bentity_type_id%5D=node&media_library_opener_parameters%5Bbundle%5D=story&media_library_opener_parameters%5Bfield_name%5D=field_hero_image&hash=6XU6pG4r_KQ6oV2O_tf71B7mh0ZP2SPMlZU5IYAVqtg&page=1&ajax_page_state%5Btheme%5D=claro&ajax_page_state%5Btheme_token%5D=uucqv3B3sql3izQWQ6M6AGJpBIo1LSwjcYwu1Jt-cKg&ajax_page_state%5Blibraries%5D=admin_toolbar/toolbar.tree%2Cadmin_toolbar/toolbar.tree.hoverintent%2Cadmin_toolbar_tools/toolbar.icon%2Cbig_pipe/big_pipe%2Cckeditor5/internal.drupal.ckeditor5%2Cckeditor5/internal.drupal.ckeditor5.emphasis%2Cckeditor5/internal.drupal.ckeditor5.htmlEngine%2Cclaro/global-styling%2Cclaro/media_library.theme%2Cclaro/node-form%2Ccontextual/drupal.contextual-links%2Ccontextual/drupal.contextual-toolbar%2Ccore/ckeditor5.autoformat%2Ccore/ckeditor5.basic%2Ccore/ckeditor5.essentials%2Ccore/ckeditor5.htmlSupport%2Ccore/ckeditor5.link%2Ccore/ckeditor5.pasteFromOffice%2Ccore/ckeditor5.sourceEditing%2Ccore/drupal.active-link%2Ccore/drupal.collapse%2Ccore/drupal.dropbutton%2Ccore/drupal.entity-form%2Ccore/drupal.form%2Ccore/drupal.states%2Ccore/drupal.tableresponsive%2Ccore/internal.jquery.form%2Ccore/normalize%2Cfield_group/core%2Cfield_group/formatter.fieldset%2Cfile/drupal.file%2Cfilter/drupal.filter%2Clinkit/ckeditor5%2Cmedia_library/ui%2Cmedia_library/widget%2Cnode/drupal.node%2Cnode/form%2Cparagraphs/drupal.paragraphs.widget%2Cpath/drupal.path%2Cpathauto/widget%2Cshortcut/drupal.shortcut%2Csystem/admin%2Csystem/base%2Ctoolbar/toolbar%2Ctoolbar/toolbar.escapeAdmin%2Ctour/tour%2Cuser/drupal.user.icons%2Cviews/views.ajax%2Cviews/views.module`
CommentFileSizeAuthor
#7 3366287-5-test-only.patch2.6 KBcatch
#5 interdiff.txt1.78 KBlauriii
#5 3366287-5.patch3.32 KBlauriii
#4 3366287-4.patch2.94 KBlauriii
#4 3366287-4-test-only.patch2.22 KBlauriii

Comments

dmurphy1 created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Title: Inserting media via the media library modal when paged redirects to the wrong destination » [regression] Inserting media via the media library modal when paged redirects to the wrong destination
Priority: Major » Critical

I can reproduce manually but haven't been able to track down the bug yet.

This bit of MediaLibrarySelectForm is a bit suspicious

    // @todo Remove in https://www.drupal.org/project/drupal/issues/2504115
    // Currently the default URL for all AJAX form elements is the current URL,
    // not the form action. This causes bugs when this form is rendered from an
    // AJAX path like /views/ajax, which cannot process AJAX form submits.
    $query = $this->view->getRequest()->query->all();
    $query[FormBuilderInterface::AJAX_FORM_REQUEST] = TRUE;
    $query['views_display_id'] = $this->view->getDisplay()->display['id'];
    $form['actions']['submit']['#ajax'] = [
      'url' => Url::fromRoute('media_library.ui'),
      'options' => [
        'query' => $query,
      ],
      'callback' => [static::class, 'updateWidget'],
      // The AJAX system automatically moves focus to the first tabbable
      // element of the modal, so we need to disable refocus on the button.
      'disable-refocus' => TRUE,
    ];
shailja179’s picture

shailja179 commented

@dmurphy1 ,
Can you give the steps to reproduce the issue?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3366287-4-test-only.patch2.22 KB
new 3366287-4.patch2.94 KB

I was able to reproduce this with the steps from the issue summary.

Here's a fix and some additional test coverage.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3366287-5.patch3.32 KB
new interdiff.txt1.78 KB

Noticed a small problem with the test changes 🙈

The last submitted patch, 4: 3366287-4-test-only.patch, failed testing. View results

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new 3366287-5-test-only.patch2.6 KB

Fresh test-only patch based on #5.

Status: Needs review » Needs work

The last submitted patch, 7: 3366287-5-test-only.patch, failed testing. View results

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

OK same error as #4, if I'd read more closely I might have realised this.

I discussed this a fair bit with @lauriii in slack, it was very hard to track down and he beat me to it, but in the end it's a very simple fix. It would be good to have more low-level test coverage of this but there's so many steps involved in testing with media library I'm not sure what that would look like, just testing the event subscriber in isolation wouldn't tell us it's doing the right thing if logic changes elsewhere.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented

+1 RTBC manually tested patch works as expected

dmurphy1’s picture

dmurphy1 commented

+1 RTBC as well. Nice work tracking this bug down!

  • larowlan committed c9af197e on 10.1.x 
    Issue #3366287 by lauriii, catch, dmurphy1: [regression] Inserting media...

  • larowlan committed d5740e4b on 11.x 
    Issue #3366287 by lauriii, catch, dmurphy1: [regression] Inserting media...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Thanks all, committed to 11.x and backported to 10.1.x

Status: Fixed » Closed (fixed)

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

rosemaryreilman’s picture

rosemaryreilman
she/her
commented

Just wanted to post that I ran into this issue again when I upgraded to 11.3.9. It's something that happened between 11.3.8 and 11.3.9.

Update: I just found this is happening on version 11.3.8 on another site. So perhaps it was actually introduced in 11.3 somewhere. This site started on 11.2.8

Update again: after further investigation the issue only seems to appear if you change the media library view to Full Pager instead of using the mini pager that it comes with. Reverting back to the mini pager seems to resolve the issue.