Broken auto creation of machine name field

According to https://www.drupal.org/node/3367037 → this is supposed to work and it is not Ajax.

Is this bug replicated on a newly-installed and unmodified Drupal site?

See the Issue tags field description for the reason I removed tags.

Can you type in the JS console: console.log(drupalSettings.machineName) and post the result here please?

#2, #4
Yes, I created a new DB, new instance of Drupal 10 as a new project with "Workflow" as the only added module. All installations were performed as instructed on https://www.drupal.org/docs/getting-started/installing-drupal/get-the-code → i.e.

• composer create-project drupal/recommended-project newD10
• Installation and configuration in browser "as usual"
• composer require drupal/workflow
• Enabling only workflow module and no submodules
• Under Admin -> Configuration -> Workflow I choose (i.e. click on button) "+ Add workflow"
• Textbox with labe "Label*" I type "Flö"
• Text to the right of the textbox says "Machine name: fl [Edit] (see attached image "fl1.png"
• Javascript console - see attached image "fl2.png"

Please note:
Since I reinstalled and rechecked... this might not be D10 issue,
it could be Workflow related since correct behavior is discovered
in other situation in D10 not related to Workflow.
In such case this might need to be moved to Workflow issue queue - could you do it or should I?

It happens on date format inputs too, thank you for the detailed steps.

Downgrading to normal since there is a workaround (editing the machine name field directly) and the feature is for admin/developers, not regular site users. This is not related to ajax so moving to the base system.

Oops, date formats? Did not see that one comming.

You are welcome (regarding detailed reporting).
Good job on spotting date format problem, excluding Ajax, downgrading and moving.

Hi,
I just pushed a commit in mr !8316 fixing this error.

First time contributing to the core, so please let me know if I didn't apply proper workflow, naming or anything else.

Ran test-only feature

1) Drupal\Tests\system\FunctionalJavascript\System\DateFormatTest::testDateFormatXss
Behat\Mink\Exception\ResponseTextException: The text "Machine name: label_with_special_characters" was not found anywhere in the text of the current page.
/builds/issue/drupal-3428652/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3428652/vendor/behat/mink/src/WebAssert.php:293
/builds/issue/drupal-3428652/core/tests/Drupal/Tests/WebAssert.php:975
/builds/issue/drupal-3428652/core/modules/system/tests/src/FunctionalJavascript/System/DateFormatTest.php:77
FAILURES!
Tests: 1, Assertions: 6, Failures: 1.

Which shows coverage.

Left 1 small comment on the MR though.

Good job for a first MR.

Prashant.c → made their first commit to this issue’s fork.

1. Resolved this https://git.drupalcode.org/project/drupal/-/merge_requests/8316#note_323107 as the constant was not required because was not being used anywhere else
2. Keeping the issue in NW as per https://www.drupal.org/project/drupal/issues/3428652#comment-15630065 🐛 Broken auto creation of machine name field Needs work
3. Tested with content type fields machine names, special characters/non-ASCII characters are being replaced properly, tested with é, à, ö, ñ, etc.

Thanks!

Weirdly enough, tests succeed on a ddev install with ddev/ddev-selenium-standalone-chrome for functionalJS tests.
I'll have a deeper look later this week, maybe there is some configuration discrepancies on my env vs drupal-ci.

Ok, so, after much contemplation and reading the docs, it seems the Test-only changes job is supposed to fail ? I feel a little silly tbh, but hey, I learnt something new !

Changing the status to need review.

Feedback on the MR appears to be addressed.

Re-ran the test-only job after the pattern change and it still fails as expected, so coverage is still good.

I think the problem is elsewhere. We're passing a regex to slugify for the allowedChars option where it expects a list of characters (so, not a regex). It's fine to do that because the replace pattern is used before sending the string to slugify.

Tried a different approach, let's see what testbot says.

the tests in machineNameTransliterationTest.js are not correct, it doesn't test how things are used in Drupal from the PHP

So sounds like the test needs to be corrected? Would almost say it should be separate from this issue and this one postponed but probably can roll the two together

Well I wanted to keep that test and fix the code instead. There is no reason to have 2 underscores in the transformed name.

added back the failing test

Oh alright :)

Oh I see... the problem is with date formats and workflow specifically because they have '([^a-z0-9_]+)|(^custom$)', and this is not going to work with what the allowedChars option does for slugify.

My guess is that the workflow module has incorrectly copy pasted the date format code as I can't see why custom would not be allowed there. But regardless date format machine names are broken in the same way.

This is going to be fun to fix in a way that supports both 'replace_pattern' => '([^a-z0-9_]+)|(^custom$)', and 'replace_pattern' => '[^a-z0-9_]+',

It's turns out that 🐛 Machine name generation is way too slow Fixed was built on an incorrect premise. That replace_pattern was always compatible with allowedChars - that's just not true. We need to do the replacing ourselves. So we transliterate and then do the replacement and do not use slugify.

well that was easier than expected :)

alexpott → changed the visibility of the branch fix/3428652d-date_format_machine_name_pattern_replacement to hidden.

alexpott → changed the visibility of the branch 3428652-broken-auto-creation to hidden.

alexpott → changed the visibility of the branch 11.x to hidden.

@nod_ I agree with the latest fix. Think we should add test coverage of the bug exposed here.

I checked that the transliteration per language is being tested and it is - in \Drupal\FunctionalJavascriptTests\MachineName\MachineNameTransliterationTest - so we have test coverage of the changes how we build the replace array. I slightly improved the test because it feels in scope.

I this issue is ready and will commit once rtbc.

Changes look good.

Tested locally on latest 11.x with standard profile install. No JS errors when adding these date formats:

Actually, one question/nit about this:

(function ($, Drupal, drupalSettings, transliterateLibrary) {
...
  const transliteratedSource = transliterateLibrary(source, { replace });
...
})(jQuery, Drupal, drupalSettings, transliterate);

Can the "transliterate" global object be passed into the the self-invoking function as "transliterate" instead of "transliterateLibrary"? It could be helpful to be able to grep for "transliterate(" for uses in JS code.

We have a method on the Drupal object called transliterate too, if you search for it now you'll find all the Drupal calls, it's to make it explicit this is not the same thing.

We have a method on the Drupal object called transliterate too, if you search for it now you'll find all the Drupal calls, it's to make it explicit this is not the same thing.

Makes sense. Can stay at RTBC.

Committed and pushed 73ec28542e to 11.x and adc4219ee6 to 11.0.x and 2ca2d354d0 to 10.4.x and 747c866073 to 10.3.x. Thanks!

Backported this to 10.3.x as a bug fix with no API concerns.

nod_ → closed merge request !8677

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