Contrib.social

editor.module's hook_entity_update() does not count file usages correctly when adding/removing images without creating new revisions

Open on Drupal.org →
Created on 5 July 2017, over 7 years ago
Updated 8 July 2024, 8 months ago

Problem/Motivation

In #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations → , @Leksat shows in comments #17.1 + #18.1 + #22 + #26 that there is another bug in editor.module's hook_entity_update(): not only does it fail in case of new/removed translations (which is what #2708411: [PP-1] editor.module's editor_file_reference filter not tracking file usage correctly for translations → is about, it also fails when adding/removing images without creating a new revision (i.e. when editing a particular revision): if images are added/removed, then the file usage is never incremented/decremented accordingly.

Note that there is NO risk of data loss! There's only risk of incorrect counting. The only risk is that files that should be deleted, aren't, because it's possible that when 0 actual uses remain, file_usage still lists >0.

Examples:

  • Start with body=''. Now edit it to body='<img>'. Usage=1, which is correct. Now edit it to body=''. Usage=0, which is also correct.
  • Start with body=''. Now edit it to body='<img>'. Usage=1, which is correct. Now edit it to body='<img><img>'. Usage=1, which is incorrect (because it already was >0, it failed to count additional new uses). Now edit it to body=''. Usage=1, which is incorrect.
  • Start with body='<img><img>'. Usage=2. Now edit it to body='<img><img><img>'. Usage=2, which is incorrect (same reason as before). Now edit it to body='<img><img>'. Usage=2, which is incorrect. Now edit it to body=''. Usage=0, which is correct (because the delta is -2, which happened to be correct).
  • Start with body='<img><img>'. Usage=2. Now edit it to body='<img><img><img>'. Usage=2, which is incorrect (same reason as before). Now edit it to body='<img><img>'. Usage=2, which is correct (but only because it's not being updated…). Now edit it to body=''. Usage=0, which is correct (because the delta is -2, which happened to result in usage=0).
  • Start with body='<img><img>'. Usage=2. Now edit it to body='<img><img><img>'. Usage=2, which is incorrect (same reason as before). Now edit it to body=''. Usage=0, which is correct (because the delta is -3, which happened to result in usage=-1, which simply results in the row being deleted).
  • Start with body='<img><img>'. Usage=2. Now edit it to body='<img>'. Usage=2, which is incorrect (same reason as before). Now edit it to body=''. Usage=1, which is incorrect (because the delta is -1).

In other words: the number of times a file is referenced is only counted correctly when going from 0 to N or from N to zero. It's not counted correctly when going from >0 to N or from N to >0.

Proposed resolution

Add test coverage to ensure that adding/removing images to a body field without creating new revisions works as expected.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Editor  →

Last updated about 1 hour ago

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024