Problem/Motivation

Discovered in #3138746: Fix 45 "shouldBeCamelCased" and related typos in core and introduced by #1894644: Unidirectional editor configuration -> filter settings syncing

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 wim_leers-comment.png35.17 KBsulfikar_s
#5 3181290-5.patch1.42 KBlongwave
#2 3181290-2.patch1.33 KBjungle
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented
Status: Active » Needs review
FileSize
3181290-2.patch1.33 KB
  • Renamed touchedBytouchedByForbiddenPropertyRule to touchedByForbiddenPropertyRule
  • Removed bytouched from core/misc/cspell/dictionary.txt
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

As far as I could see these lines do nothing at all so we should probably just remove them.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Acro Commerce, Thunder commented
Status: Needs review » Needs work

I agree with #3 - it might be good to have a look and see what happened on #1894644: Unidirectional editor configuration -> filter settings syncing

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
3181290-5.patch1.42 KB

The patch this property was introduced in was #1894644-52: Unidirectional editor configuration -> filter settings syncing, the interdiff is https://www.drupal.org/files/interdiff_6257.txt

There are several references to touchedByAllowedPropertyRule in this interdiff but nothing for the similarly-named ForbiddenPropertyRule. Therefore I am certain this is dead code introduced while that patch was being worked on, but that never made it into the final version.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented
Issue tags: +Needs subsystem maintainer review

Tagging "Needs subsystem maintainer review" if extra reviews are needed

sulfikar_s’s picture

sulfikar_s CreditAttribution: sulfikar_s at Zyxware Technologies commented
Status: Needs review » Reviewed & tested by the community
FileSize
wim_leers-comment.png35.17 KB

Hello all, the patch in #5 applied cleanly. 

Checking patch core/misc/cspell/dictionary.txt...
Checking patch core/modules/editor/js/editor.admin.es6.js...
Checking patch core/modules/editor/js/editor.admin.js...
Applied patch core/misc/cspell/dictionary.txt cleanly.
Applied patch core/modules/editor/js/editor.admin.es6.js cleanly.
Applied patch core/modules/editor/js/editor.admin.js cleanly.

Also I've verified that it is discovered in the #1894644: Unidirectional editor configuration -> filter settings syncing and I agree with what @longwave said as there is no references found for 'touchedBytouchedByForbiddenPropertyRule'. So, +1 to removing it.

@Wim Leers added the forbiddenPropertyRule as a part of his strategy, he mentioned the same in his comment on #1894644: Unidirectional editor configuration -> filter settings syncing, you can see that part below,

wim-leers comment

but as @longwave said,

it is a dead code introduced while that patch was being worked on, but that never made it into the final version.

So RTBC.

  • catch committed 70c1f6c on 9.2.x 
    Issue #3181290 by jungle, longwave, sulfikar_s: Remove dead code in...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 70c1f6c and pushed to 9.2.x. Thanks!

Status: Fixed » Closed (fixed)

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