Problem/Motivation

username has a character limit of 60. If we create a user with name of 58 or 59 characters and try to translate a content, translation will not get saved. However on creating or editing a content in original language, it gets saved.

In authoring information tab (in node add, edit), 'Authored by' has maxlength 1024. Only in translation form, Authored by inside translation tab has maxlength 60. In the same translation form, Authored by field inside 'Authoring information' tab has maxlength 1024 and this causes issue.

The ContentTranslationHandler adds an entity_autocomplete field with a maxlength of 60 characters to the node edit form. The intent is probably to match the value in use on the UserInterface::USERNAME_MAX_LENGTH. However the autocomplete selection adds the (nid) to the selected username. If the value is greater than the limit it prevents submit the node edit form.

Steps to reproduce

Set a entity type as translatable. Create a user with a very long username (ex: 59 characters).
Create an entity with this user and save it.
Create a translation and try to save it.

Proposed resolution

Remove the maxlength attribute on the field.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#29 3473029-29-10.5.x.diff2.65 KBherved
#14 before-after-comparison.jpg544.88 KBigorgoncalves
#3 node add form in original language.png139.66 KBannmarysruthy

Issue fork drupal-3473029

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:

About issue forks

Comments

guillaumepacilly created an issue. See original summary.

annmarysruthy made their first commit to this issue’s fork.

annmarysruthy’s picture

annmarysruthy commented
StatusFileSize
new node add form in original language.png139.66 KB

In authoring information tab (in node add, edit), 'Authored by' has maxlength 1024. Only in translation form, Authored by inside translation tab has maxlength 60. In the same translation form, Authored by field inside 'Authoring information' tab has maxlength 1024 and this causes issue.

Instead of removing maxlength, I would recommend changing maxlength of 'Authored By' inside translation tab to 1024 and make it same as ' Authored by' field inside 'Authoring information' tab

annmarysruthy’s picture

annmarysruthy commented
Version: 10.3.x-dev » 11.0.x-dev

annmarysruthy changed the visibility of the branch 3473029-wrong-maxlength-on to hidden.

quietone’s picture

quietone commented
Version: 11.0.x-dev » 11.x-dev

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

annmarysruthy changed the visibility of the branch 3473029- to hidden.

annmarysruthy opened merge request !9534

annmarysruthy’s picture

annmarysruthy commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests

New approach will need issue summary update.

Also might be nice to get a test showing how this is a probelm.

annmarysruthy’s picture

annmarysruthy commented
Issue summary: View changes

dcam made their first commit to this issue’s fork.

dcam’s picture

dcam commented
Status: Needs work » Needs review
Issue tags: -Needs issue summary update, -Needs tests

I added a test. The IS was updated in #11.

igorgoncalves’s picture

igorgoncalves commented
StatusFileSize
new before-after-comparison.jpg544.88 KB

Thanks @dcam

I just followed the steps to reproduce and i confirm the error at the time to create my test content translation.

And after applying the patch, i could create the translation with no problem.

Drupal version 11.2.0-dev

before and after

smustgrave’s picture

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

Pulled up the original issue #1807800: Add status and authoring information as generic entity translation metadata and don't see any discussion around using 60 so I imagine no one is super against changing.

Test-only job didn't run https://git.drupalcode.org/issue/drupal-3473029/-/jobs/5575179

Ran locally but it seems to be passing so may need some tweaking

dcam opened merge request !12393

dcam’s picture

dcam commented
Status: Needs work » Needs review

I asked in the core-development Slack if the test-only job passes because the new assertions are in an abstract base class. Per @godotislate that is the case. The test-only runner looks for changes in *Test.php files. I was advised to create a test-only MR, which I did in MR 12393.

dcam’s picture

dcam commented

And now you can see that multiple content translation UI tests are failing. Thank you @godotislate!

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Good to know, but the test failure happening doesn't appear to happen around the code being introduced.

Also

    $this->assertSession()->statusMessageNotExists('error');

This could probably be updated.

dcam’s picture

dcam commented
Status: Needs work » Needs review

I improved the test. I think it's probably good to go now.

smustgrave changed the visibility of the branch 3473029-correct-maxlength-test-only to hidden.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Hiding the test-only branch incase the bot picks it up.

But I see that the failures are now coming from the added test coverage

│ /builds/issue/drupal-3473029/core/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php:368

That was the only feedback I had, so believe this one is good to go.

longwave closed merge request !9534

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Backported to 11.2.x as an eligible bug fix. This could also go to 10.5.x/10.6.x but doesn't cherry-pick cleanly; if anyone feels strongly about that please reopen with a backport MR against 10.6.x.

Committed and pushed 970ce375723 to 11.x and 6d8c0395f8f to 11.2.x. Thanks!

  • longwave committed 6d8c0395 on 11.2.x 
    Issue #3473029 by dcam, annmarysruthy, igorgoncalves, smustgrave,...

  • longwave committed 970ce375 on 11.x 
    Issue #3473029 by dcam, annmarysruthy, igorgoncalves, smustgrave,...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Status: Fixed » Closed (fixed)

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

herved’s picture

herved commented
StatusFileSize
new 3473029-29-10.5.x.diff2.65 KB

Patch for 10.5.x if anyone else needs it