- 🇬🇧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 6:58pm 15 March 2023 - 🇨🇦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
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
. The last submitted patch, 16: core-2978498-16-testonly.patch, failed testing. View results →
- Status changed to RTBC
over 1 year ago 10:57pm 15 March 2023 - 🇺🇸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.
- Status changed to Fixed
over 1 year ago 8:05am 17 March 2023 - 🇬🇧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 9:04am 21 March 2023 - 🇫🇮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 11:27am 21 March 2023 - Status changed to Needs work
over 1 year ago 11:30am 21 March 2023 - 🇬🇧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 2:49am 27 March 2023 - Status changed to Needs work
over 1 year ago 12:25am 28 March 2023 - 🇺🇸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 forcore/modules/system/js/system.modules.js
hassources.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 hasI 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
- Status changed to Fixed
5 days ago 10:01am 29 November 2024 - 🇬🇧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.