Using the module filter on the module list page a module can only be found by system name if the description ends in a dot

Created on 9 June 2018, over 6 years ago
Updated 25 January 2023, almost 2 years ago

Problem

Using the module filter on the module list page a module can only be found by system name if the description ends in a dot. This is caused by the fact that jQuery's .text() method doesn't add any spaces. Therefore the Regex in Drupal.behaviors.tableFilterByText showModuleRow which uses word boundaries only recognizes the word if the description contains a period to end the sentence and make the machine name its own word.

This is why for example searching for "dblog" works currently but searching for "yoast" doesn't show the Real-Time SEO module in the 8.x-2.0-alpha3 release.

Proposed Solution

Make the showModuleRow slightly more elaborate by ensuring the values of the different possible search sources are separated by a space.

🐛 Bug report
Status

Needs work

Version

10.1

Component
System 

Last updated 4 days ago

No maintainer
Created by

🇳🇱Netherlands kingdutch

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇬🇧United Kingdom jonathan1055

    This is still an error in 10.1. The javascript has changed but the fault was replicated.
    @kingdutch - this issue is assigned to you. Would you like to work on it? If not then I will.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇨🇦Canada star-szr

    I ran into this as well, coming from 📌 Add optional parameter to Nightwatch drupalInstallModule command to enable module dependencies Fixed .

    New patch, so no interdiff. Test-only, then test + fix.

  • 🇨🇦Canada star-szr

    Updating the issue summary a bit.

  • 🇨🇦Canada star-szr

    I want to add that we could certainly add a test module that is dedicated to testing this type of case, and I can do so if requested rather than arbitrarily using comment_base_field_test.

  • Status changed to RTBC over 1 year ago
  • 🇺🇸United States smustgrave

    Verified the issue by removing the period from the description of dblog
    Searching for dblog didn't return anything.

    Apply patch
    Searching for dblog returns the module even without a period at the end of the description.

  • 🇬🇧United Kingdom jonathan1055

    Now that this is marked as RTBC there's a flag by the test-only patch saying it will be retested every two days. But that's not going to help anything, as we know it fails (by design). So I'm hiding that patch, just to see if the good one gets tagged with automated retesting instead.

    • lauriii committed 753eef8a on 10.1.x
      Issue #2978498 by Cottser, Kingdutch: Using the module filter on the...
  • Status changed to Fixed over 1 year ago
  • 🇫🇮Finland lauriii Finland

    Thanks for the fix @Cottser. Happy to see you in the queues again! 👋😊

    Committed 753eef8 and pushed to 10.1.x. Thanks!

  • 🇬🇧United Kingdom jonathan1055

    Thanks for the commit. Would you consider backporting to 9.x? This is such a simple fix to make and I'd be happy to do a MR with the same changes if no one else wants to? (can't be cherry-picked directly because the js file is very slightly different)

  • Status changed to Needs work over 1 year ago
  • 🇫🇮Finland lauriii Finland

    I didn't originally backport this because I thought there could be a small risk of some Nightwatch tests failing in contrib because this might make modules visible in the searches that weren't visible before. That said, I don't think this is huge risk and there aren't that many contributed modules using Nightwatch. Based on that, I think we could backport this.

  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work over 1 year ago
  • 🇬🇧United Kingdom jonathan1055

    @Rishabh Vishwakarma I had just created a new issue branch and was working on this. The implication from #24 was that I would do this work unless someone volunteered. So I have just wasted my time now. I guess I should have assigned this issue to myself before starting it. Or you should have.

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India gauravvvv Delhi, India

    Re-rolled patch #16, for 9.5.x

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    @jonathan1055 what MR? If there is one we should be doing the fix there.

    Patch #28 dropped the test cases.

  • 🇮🇳India gauravvvv Delhi, India

    Patch #28 dropped the test cases.

    We don't have test case file in 9.5.x core/modules/system/tests/src/FunctionalJavascript/ModuleFilterTest.php

  • 🇬🇧United Kingdom jonathan1055

    We don't have test case file in 9.5.x core/modules/system/tests/src/FunctionalJavascript/ModuleFilterTest.php

    That's correct, and it is what I was doing as a follow-up to #24 and #25 when the new patch was uploaded in #26. I had created the issue branch and was working locally but had not pushed it up to d.o. so not made a MR yet. But I stopped work when I thought that @Rishabh Vishwakarma was taking on the tasks.

    Also, just a quick question for @Gauravvvv, patch #28 is not precisely a re-roll of #16.
    In #16 the change for core/modules/system/js/system.modules.js has

               sources.forEach((item) => {
    -            sourcesConcat += item.textContent;
    +            sourcesConcat += ` ${item.textContent}`;
               });
    

    but in patch #28 this is became

               sources.forEach(function (item) {
    -            sourcesConcat += item.textContent;
    +            sourcesConcat += " ".concat(item.textContent);
               });
     

    There may be a good reason for this, but I just want to know. We should aim to keep 9.x code in line with 10.x code where possible as that makes future maintainance and backporting easier.

  • 🇮🇳India gauravvvv Delhi, India

    Also, just a quick question for @Gauravvvv, patch #28 is not precisely a re-roll of #16.
    In #16 the change for core/modules/system/js/system.modules.js has

    I have not made any chnages manually into core/modules/system/js/system.modules.js file. All these changes are compiled by yarn.

    Also both of these lines of code achieve the same result, which is concatenating a space character and the text content of the item variable to the sourcesConcat variable

  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Status changed to Fixed 5 days ago
  • 🇬🇧United Kingdom jonathan1055

    The change was committed to 10.1.x in March 2023. It was not back-ported to D9 but that does not matter now, so I think this can be marked as 'Fixed'

  • 🇬🇧United Kingdom jonathan1055

    Shorter title.
    Adding parent issue for reference.

Production build 0.71.5 2024