Problem/Motivation

The patch from #2903524: Don't add quickedit to displays where entities have a forward revision. removes the data-quickedit-entity-id attribute from non-latest revisions, but always adds the data-quickedit-field-id attribute even if the revision being viewed is not the latest revision. This leads to the JS error "Quick Edit could not associate the rendered entity field markup" when viewing a non-latest default revision, because the entity markup has data-quickedit-field-id attributes but no parent data-quickedit-entity-id attribute.

Replication steps:

  1. Install the Standard profile
  2. Enable Content Moderation
  3. Enable the Editorial Workflow for Articles
  4. Create a published Article
  5. Create a draft of that Article
  6. View the default revision of the Article
  7. See a JS error in your console

I'm setting the priority of this to Major to match #2903524: Don't add quickedit to displays where entities have a forward revision.. In my opinion 8.4 should not be released until this is fixed.

Proposed resolution

Remove the data-quickedit-field-id attribute on non-latest revisions.

Remaining tasks

Review provided patch.

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

samuel.mortenson created an issue. See original summary.

samuel.mortenson’s picture

Status: Active » Needs review
StatusFileSize
new2.05 KB
sam152’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me.

wim leers’s picture

Title: Javascript errors thrown when viewing non-latest default revision of entity » JavaScript errors thrown when viewing non-latest default revision of entity

👍

It'd be good to see a failing test-only patch though.

wim leers’s picture

Also note this is essentially a follow-up for #2903524: Don't add quickedit to displays where entities have a forward revision., this patch shows we didn't completely solve the "Quick Edit + Content Moderation" integration problems there.

catch’s picture

StatusFileSize
new1.08 KB
new2.05 KB

Test-only patch + the original to keep the issue green.

The last submitted patch, 6: 2910005-tests-only.patch, failed testing. View results

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2910005-1.patch, failed testing. View results

catch’s picture

Status: Needs work » Reviewed & tested by the community

umm

lauriii’s picture

#4 includes a easty to understand explanation of the nature of this bug fix, thanks @Wim Leers!

This seems like a non-risky issue which potentially could be backported to 8.4.x. I'm waiting for confirmation from another committer before cherry picking.

Committed 4dee162 and pushed to 8.5.x. Thanks!

  • lauriii committed 4dee162 on 8.5.x
    Issue #2910005 by catch, samuel.mortenson: JavaScript errors thrown when...
catch’s picture

Agreed we can cherry-pick this to 8.4.x once it's back open now that 8.4.0 is out.

catch’s picture

Status: Reviewed & tested by the community » Fixed

  • catch committed 8002f65 on 8.4.x authored by lauriii
    Issue #2910005 by catch, samuel.mortenson: JavaScript errors thrown when...

Status: Fixed » Closed (fixed)

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