The node translation handler changes the node edit form.
It adds a translation message to the submit button, but also does this to the publish and unpublish button which are removed in #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button

 if (isset($status_translatable)) {
        foreach (['publish', 'unpublish', 'submit'] as $button) {
          if (isset($form['actions'][$button])) {
            $form['actions'][$button]['#value'] .= ' ' . ($status_translatable ? t('(this translation)') : t('(all translations)'));
          }
        }
      }
CommentFileSizeAuthor
#3 translation-old-buttons-2917031-3.patch896 bytesmallezie

Comments

mallezie created an issue. See original summary.

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Issue summary: View changes
mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Status: Active » Needs review
StatusFileSize
new translation-old-buttons-2917031-3.patch896 bytes

And a patch

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented

I guess this shows we didn't have suitable test coverage for this.

Also, maybe this should be abstracted out of Node and added for all translatable entities.

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented

Not sure about the test-coverage.

It's actually just dead code. There is no publish and unpublish button anymore so the value is never adjusted of that button.
I actually found out about the code through #2915231: Deprecated action buttons in node form which is a 'bug' in rabbit_hole wich assumes those buttons are their, and adds some elements to it, making the above code do run and throw a warning.

About the abstraction, seems we can up that to the ContentTranslationHandler class, but then we would add that label to all translatable content entities, which doesn't sound bad, but is a change if i'm not wrong.
It was done in the nodetranslationhandler since that was the one with all 3 buttons (submit / publish / unpublish) at least before the drop button removal.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented
Status: Needs review » Reviewed & tested by the community

I guess we can add a follow up for the abstraction part.

catch’s picture

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

Committed 1a7ba41 and pushed to 8.5.x. Thanks!

  • catch committed 0f42cd0 on 8.5.x 
    Issue #2917031 by mallezie: NodeTranslationHandler references old...
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented
Version: 8.5.x-dev » 8.4.x-dev
Issue tags: +Needs committer feedback

I think this should be cherry picked to 8.4.x because we're seeing bug reports of the same issue. For example #2918433: Notice: Undefined index: #value in Drupal\node\NodeTranslationHandler->entityFormAlter().

Status: Fixed » Closed (fixed)

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

amateescu’s picture

amateescu commented
Category: Task » Bug report
Status: Closed (fixed) » Reviewed & tested by the community

If this patch is fixing #2918433: Notice: Undefined index: #value in Drupal\node\NodeTranslationHandler->entityFormAlter() than this is bug, not a task, so we have to treat it as such and backport it to 8.4.x.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: translation-old-buttons-2917031-3.patch, failed testing. View results

slydevil’s picture

slydevil commented
Status: Needs work » Fixed

Issue #2918433 can also use the attached patch as it applies to the 8.4 code base cleanly. Setting this to fixed again.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented
Status: Fixed » Reviewed & tested by the community

I still think we should cherry pick this issue back to 8.4.x

slydevil’s picture

slydevil commented

Ok, I've added the exact same patch to the other issue...and re-opened.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Cherry-picked to 8.4.x. Thanks!

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented

Thank you @xjm.

  • xjm committed b0126e0 on 8.4.x authored by catch 
    Issue #2917031 by mallezie: NodeTranslationHandler references old...

Status: Fixed » Closed (fixed)

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