Private property in extensible class AMP_Base_Sanitizer

[schlessera]

The extensible class AMP_Base_Sanitizer has a private property $should_not_removed_nodes [sic].

It is being used in a public method remove_invalid_child(), and if an extending class would override this public method, for example by copying the source code over and making modifications, this might not behave as expected. Copying the code as is from the base method would create a separate, dynamic property with a lifetime independent of the original one, and the code would act either on one or the other, depending on what class it comes from.

Is the use of private intentional here? If yes, would it make sense to make the remove_invalid_child() final to make sure no one overrides it, introducing hard-to-diagnose bugs?

If not, it might make sense to turn the property from private to protected, so it behaves more consistently with regards to inheritance.

[westonruter]

Good point. Yes, the method should be marked final.

As part of #2315 it may make sense for this to be protected when the method gets reincarnated.

[westonruter]

The extensible class AMP_Base_Sanitizer has a private property $should_not_removed_nodes [sic].

By the way, do you have a problem with my command of the English language‽ 025d1bb 😉

[amedina]

@schlessera @westonruter it seems this is an easy fix? Can we go ahead with it?