In additem.pl, in the code that generates item form you find : @values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && defined($itemrecord->field($tag)->subfield($subtag))); This code is missing the check of $itemrecord->field($tag) undef. I did not found a way to test it because item tag is always defined when editing an item, but it would be safer/prettier to add this check.
Created attachment 18724 [details] [review] Proposed patch
Created attachment 18737 [details] [review] Bug 10428 - undefined check in add additem.pl In additem.pl, in the code that generates item form you find : @values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && defined($itemrecord->field($tag)->subfield($subtag))); This patch adds the check of $itemrecord->field($tag) undef. I did not found a way to test it because item tag is always defined when editing an item, but it would be safer/prettier to add this check. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Agreed. Not easy to test, but this is a completely sensible additional test.
Created attachment 18814 [details] [review] Bug 10428 - undefined check in add additem.pl In additem.pl, in the code that generates item form you find : @values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && defined($itemrecord->field($tag)->subfield($subtag))); This patch adds the check of $itemrecord->field($tag) undef. I did not found a way to test it because item tag is always defined when editing an item, but it would be safer/prettier to add this check. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passed-QA-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to comment #0) > In additem.pl, in the code that generates item form you find : > @values = $itemrecord->field($tag)->subfield($subtag) if ($itemrecord && > defined($itemrecord->field($tag)->subfield($subtag))); > > This code is missing the check of $itemrecord->field($tag) undef. > I did not found a way to test it because item tag is always defined when > editing an item, but it would be safer/prettier to add this check. Just double-checking: did you run into any situation where the absence of the test resulted in a user-visible problem? It occurs to me that intentionally damaging the contents of the LastCreatedItem cookie would be a way of exercising this patch.
Pushed to master. Thanks, Fridolyn!
This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Fridolyn!
Pushed to 3.10.x, will be in 3.10.10
Pushed to 3.8.x, will be in 3.8.17