There are several places in commerce where an Equal operator is used to compare the #value of an element trigger where an Identical operator would be better since something like if ($form_state['triggering_element']['#value'] == t('Add line item')) can evaluate to true if #value of the triggering_element is (int) 0 or (boolean) TRUE. A good example is if un-checking a checkbox triggers the AJAX callback.
This is bad for other modules that integrate AJAX callbacks into the form that have submit handlers as they may never get called if $form_state['rebuild'] is set to TRUE as a result of the improper evaluation in the if.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | commerce-use_identical_operators_in_ajax-2652858-0.patch | 2.77 KB | acrazyanimal |
Comments
Comment #2
acrazyanimal commentedI searched the module for all such situations and came across three. Here is a patch which is git aware.
Btw, my use case to get this fixed is to provide a proper embedded registration management section on the order edit ui for the commerce_registration module. This will greatly improve the admin functionality for commerce_registration.
Comment #3
acrazyanimal commentedComment #4
mglamanInteresting! I wonder if there is a way we could write a test for this, but seems like only way to test is by implementing a form alter, which tests cannot do without a testing module (that we do not have.)
Comment #5
TimRutherford commentedCan confirm patch #2 works.
As mentioned above, testing this case seems to be a bit tricky. Not really an easy was to change the triggering element in tests without somehow adding elements to the form. Might be best to just leave the tests alone.
Comment #6
rszrama commentedThanks for the review. Committed!