Backwards compatibility issue in InputfieldWrapper

[teppokoivula]

Hi Ryan! It appears that commit 132ea07 and more specifically the addition of a new __get() method to InputfieldWrapper resulted in unexpected issues with the VersionControl module -- and potentially elsewhere too.

In this case I was asking for the "id" field from InputfieldForm with $form->id. Before this change that would return a child element matching the given key, now it first attempts to get the given key from the parent element and only returns a child element if parent had no such key or it's value was null.

I don't know why exactly this particular change was made so there might not be a clean way around it, but just thought you should know. I'm not sure how widespread the effects of this are, but potentially they could break random pieces of code here and there.

[ryancramerdesign]

I don't have a good answer yet here but will be looking more. I've found using that direct access method to be problematic when it comes to Inputfield forms that field names that can collide with attribute names (or setting names). Not typically a problem with things like module or field configuration, where the developer controls all the field names. But more a problem with things like FormBuilder where users define the field names.

The issue is that any given Inputfield can have children, attributes, and settings, plus direct access API vars. When there is a collision (like the one you identified, id attribute vs. field named 'id'), that's a problem because code written to assume $field->id means the id attribute will fail, and code written to assume $field->id means some child element will fail in other cases. So basically, there's potential for it to be broken either way. Since you can set a setting or attribute via direct access, but not a field, I thought it was important that the corresponding __get referred to the same thing that the __set would. But in the end, if dealing with a form that's going to have arbitrary field names outside of the developers control, I recommend avoiding direct __get accesses with Inputfields, except for setting attributes and settings. It's preferable to use these methods in cases where the Inputfield names may be outside of your control:

• $field->attr() for getting or setting attributes.
• $field->getSetting() for getting settings and direct access for setting them.
• $field->getChildByName() or $field->get() for retrieving a field.

[teppokoivula]

I agree with everything you've said there, but to be honest that doesn't actually change the nature of this issue: the implementation changed in that commit, and as a result code that used to work could now return unexpected values. In other words backwards compatibility was broken.

I'm going to leave this entirely to your discretion. I get that sometimes the API needs to change, and it's entirely understandable if this is one of those cases.