Having a machine name finishing with "_" is just weird. Here is a patch to fix that.

Files: 
CommentFileSizeAuthor
#20 test_fail_when_more_than_two_cases.patch802 bytesvaplas
#19 core-js-machine-name-1643386-19.patch1.4 KBvaplas
#17 core-js-machine-name-1643386-17.patch644 bytesvaplas
#6 core-js-machine-name-1643386-6.patch596 bytesnod_
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core-js-machine-name-1643386-6.patch. Unable to apply patch. See the log in the details link for more information. View
core-js-trim-machine_name.patch533 bytesnod_
PASSED: [[SimpleTest]]: [MySQL] 37,027 pass(es). View

Comments

nod_’s picture

Status: Active » Needs review
SebCorbin’s picture

Status: Needs review » Reviewed & tested by the community

Simple patch, works flawlessly for me.

chx’s picture

Status: Reviewed & tested by the community » Needs review

Thanks for the patch. But don't we want to do this on the server side too / instead?

nod_’s picture

There might be people that wants to be able to add any number of underscore in their machine name at the end. I don't think we should mess with what people can do when they willingly edit the machine name and add one or more underscore.

It's just that by default, this JS function should trim before "machine-naming", it's an entirely cosmetic thing and as such should only live on the front-end. If there is a piece of PHP that's supposed to do the same kind of processing in core it should trim as well, but i don't know if there is and where it is.

chx’s picture

Hrm. trim(). What if I convert Yahoo! to a machine name? Do we want yahoo or yahoo_?

nod_’s picture

Title: Trim machine name to avoid useless "_" at the end » Avoid useless "_" at begining and end of JS machine name transliterate
FileSize
596 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core-js-machine-name-1643386-6.patch. Unable to apply patch. See the log in the details link for more information. View

That's a good one. Patch updated

SebCorbin’s picture

All good here

droplet’s picture

Status: Needs review » Needs work

It will be no EDIT link for CJK inputs

nod_’s picture

Actually that's one of your issues #1447860: Show machine name input if every character is replaced :)

Would you say fixing the other issue would solve #8 as well?

droplet’s picture

Yeah. My issue able to fix this problem. But seems no one interested my idea there.

So if this issue get committed, it will broken the field UI :(

nod_’s picture

Status: Needs work » Postponed
Pancho’s picture

Status: Postponed » Needs review

Re #5:

Hrm. trim(). What if I convert Yahoo! to a machine name? Do we want yahoo or yahoo_?

I'd say in this case we want 'yahoo' only. The trailing underscore doesn't add any valuable information, yet ugliness. This is not what the user wants in more than 80% of use cases.
If the underscore really makes a difference, it can still be added manually to the machine name.

Also, re #10:
Why would this break anything that's not yet broken?
Currently, if all characters are substituted, the resulting machine name will be empty, unless manually set, therefore failing validation. After the patch this remains true.
So while this issue does fix it for human-readable names that contain at least one valid character, the other one would fix the case of no character being empty. Doesn't seem to break things.

Status: Needs review » Needs work

The last submitted patch, 6: core-js-machine-name-1643386-6.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

vaplas’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
644 bytes

Problem still exists. Update #6 with two and half changes:

1. transfer "trim" from transliterate to machineNameHandler, because it is not problem of transliterate, but field label UX.

2. change order processing string: first - "trim start _", second - "substring by max length", third - "trim end _", it important for long text.

2,5. change "_*" on "_+" in trim regexp.

droplet’s picture

Issue tags: +JavaScript, +Needs JS testing
vaplas’s picture

vaplas’s picture

In #19 i used exist case instead creating new. Why? Because when test have more than 2 cases it works bad. For example, just duplicate first case.

Status: Needs review » Needs work

The last submitted patch, 20: test_fail_when_more_than_two_cases.patch, failed testing.

vaplas’s picture

Split it to own issue

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

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