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 or other word boundary separator. This is caused by the fact that we concatenate together the module description and machine name without adding 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 accurate by ensuring the values of the different possible search sources are separated by a space.

Issue fork drupal-2978498

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Kingdutch created an issue. See original summary.

kingdutch’s picture

The attached patch uses map (es5 feature with wide support: https://caniuse.com/#feat=es5) to return the text of each element which can then be combined with a join using a space. This ensures there's no hidden requirements on the description field for the system name to be searchable.

Added related issue for the uninstall page that may suffer from a similar issue.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

Status: Needs review » Postponed (maintainer needs more info)

> 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.

I don't understand why the problem happens. What is different about 'dblog' and 'yoast'? Can you explain in more detail please?

kingdutch’s picture

Status: Postponed (maintainer needs more info) » Active

The description for the Real-Time SEO module is:

"Adds Real-time SEO page analysis and configuration"

This description does not end in a period. For historic reasons the module has a different name from its system name (system name wasn't changed for BC reasons). Because the description does not end with a period, when I search for the term "yoast" (part of the module system name) then this will not match because the search string contains "Adds Real-time SEO page analysis and configurationyoast" which puts "configurationyoast" together.

The description for the dblog module is:

"Logs and records system events to the database."

Neither the module name nor the description contains the word "dblog". However, when you search for "dblog" in the module filter then the search text that is used is "Logs and records system events to the database.dblog". The current search uses word boundaries and the dot in the module description causes "dblog" to be a separate word. This causes the dblog module to be findable for its system name.

------

The minimal module that could reproduce not being searchable by system name:

name (system name) - Description not ending in a period
Some Module (different) - This is a module that can not be found by its system name

The same module searchable by system name:
name (system name) - Description ending in a period
Some Module (different) - This is a module that can be found by its system name.

The patch ensures that both of the above cases can be found.

kingdutch’s picture

Assigned: kingdutch » Unassigned
Status: Active » Needs review

Oops. Patch is attached so status should be Needs Review.

kingdutch’s picture

Assigned: Unassigned » kingdutch
Status: Needs review » Needs work
Issue tags: +Needs test

We now have automated Javascript testing that can help me demonstrate this and show that the fix works. I'll take a crack at it this weekend or next week :)

avpaderno’s picture

Issue tags: -Needs test +Needs tests

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

jonathan1055’s picture

Version: 9.5.x-dev » 10.1.x-dev

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.

star-szr’s picture

Assigned: kingdutch » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests
Related issues: +#3264940: Add optional parameter to Nightwatch drupalInstallModule command to enable module dependencies
StatusFileSize
new1.17 KB
new1.97 KB

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

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

star-szr’s picture

Issue summary: View changes

Updating the issue summary a bit.

star-szr’s picture

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

The last submitted patch, 16: core-2978498-16-testonly.patch, failed testing. View results

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

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.

jonathan1055’s picture

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.


[edit - yes that seemed to work, the flag is now on the good patch]

  • lauriii committed 753eef8a on 10.1.x
    Issue #2978498 by Cottser, Kingdutch: Using the module filter on the...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

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

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

jonathan1055’s picture

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)

lauriii’s picture

Version: 10.1.x-dev » 9.5.x-dev
Status: Fixed » Needs work

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.

rishabh vishwakarma’s picture

Status: Needs work » Needs review
StatusFileSize
new1.34 KB

Adding patch for 9.5.x

jonathan1055’s picture

Status: Needs review » Needs work
Issue tags: -Needs Review Queue Initiative

@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. I was also working on adding test coverage at 9.5.

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new1.54 KB

Re-rolled patch #16, for 9.5.x

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

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

Patch #28 dropped the test cases.

gauravvvv’s picture

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

jonathan1055’s picture

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.

gauravvvv’s picture

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

avpaderno’s picture

Title: 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 » Using the module filter on the module list page a module can only be found by system name if the description ends with a period

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

jonathan1055’s picture

Version: 11.x-dev » 10.1.x-dev
Status: Needs work » Fixed

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'

jonathan1055’s picture

Title: Using the module filter on the module list page a module can only be found by system name if the description ends with a period » Module filter only matches with system name if the description ends in a period
Parent issue: » #3015494: Make admin list filters behave more consistently.

Shorter title.
Adding parent issue for reference.

Status: Fixed » Closed (fixed)

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