Problem/Motivation

When selecting an address at checkout, checkbox fields get always checked, even when they shouldn't be: when the saved value for the checkbox field of the address is 0 (= "unchecked").

This bug does not appear in the 7.x-1.0-alpha1 version (well, technically it is in there, but isn't notable, it's a so called "sleeping" bug in there ;)).

How to reproduce:

  1. Add an address field of type "checkbox". One way of doing this is by implementing a field and a field handler using the Ubercart Addresses field handler API, but it would be easier to add the field with the Extra Fields Pane module.
  2. Add an address to the address book and leave the checkbox unchecked.
  3. Add a product to the cart and go to checkout.
  4. In the delivery or billing pane, select the address from the address book which had the checkbox unchecked.

Expected result:

The checkbox is not checked.

Actual result:

The checkbox is checked.

Affected versions

7.x-1.x version only.

Debug information

This issue is indirectly caused by Drupal core. The saved value for the checkbox field is 0, but Drupal assumes that a posted value of 0 for a checkbox means the checkbox is checked. This is because some browsers post a value of 0 for a checked checkbox (let me guess, Internet Explorer? Oops, sorry for the bashing :$).
See also the documentation about form_type_checkbox_value().

Proposed resolution

I see two ways of fixing this:

  • Convert saved values for checkboxes to booleans before applying the address on the checkout form. This should be easy to implement, now that fields should specify their data type. See #1910626: Ubercart Addresses field handler API: Fields should specify their data type. Values can be converted to booleans for all fields that claim to be of a boolean type.
  • Rewrite the code for applying addresses a bit. Now just the form state's input values are overwritten when applying an address, which makes Drupal believe the values were filled in manually.

Remaining tasks

  • Investigate the possible fixes.
  • Implement a fix.

User interface changes

Not much, just that checkboxes won't be checked when they weren't checked for the saved address.

API changes

Not sure yet, probably none.

I'm working on it.

Files: 
CommentFileSizeAuthor
#3 uc_addresses7-checkbox-bug-address-select-1927258-3.patch1.15 KBMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 2,801 pass(es).
[ View ]
#2 uc_addresses7-checkbox-bug-address-select-1927258-1.patch1.04 KBMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 3,387 pass(es).
[ View ]
#1 uc_addresses7-uc_order-and-user-address-properties-1900468-9.patch3.28 KBMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 3,387 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new3.28 KB
PASSED: [[SimpleTest]]: [MySQL] 3,387 pass(es).
[ View ]

This is my first attempt for fixing this bug. When tested in combination with Extra Fields Pane, be sure to apply the patch from #1926200: Fields should specify their data type first (on Extra Fields Pane).
This patch just converts address values that should be boolean to a boolean when an address is about to be applied (on the checkout form).

I'm thinking: maybe converting values to booleans should happen in UcAddressesSchemaAddress instead? Also, I should investigate the possible other solution (not overwrite form state's input). An automated test would also be nice, if possible, though this requires modifications in the test module.

StatusFileSize
new1.04 KB
PASSED: [[SimpleTest]]: [MySQL] 3,387 pass(es).
[ View ]

Woops, wrong patch posted. Here's the right one.

StatusFileSize
new1.15 KB
PASSED: [[SimpleTest]]: [MySQL] 2,801 pass(es).
[ View ]

This patch adjust the UcAddressesSchemaAddress and makes sure the value is converted to the right data type before the field is set. This looks like it's a more generic solution, though it won't convert data of a type it doesn't know of. I have examined hook_entity_property_info() to see what data types I could expect.

Let's see what the testbot says.

Version:7.x-1.x-dev» 6.x-2.x-dev
Status:Needs review» Patch (to be ported)

The testbot says that this change doesn't break anything. An automated test to ensure the bug is fixed would be great, but this means quite some changes in the test module for only little benefit. As I want to reach a deadline (see #1910860: Ubercart Addresses roadmap), I'd better spent my time on other issues.
Committed #3.

Could use a backport to 6.x-2.x for consistency.

Status:Patch (to be ported)» Needs review

The patch in #3 applies also on the 6.x-2.x branch.

#3: uc_addresses7-checkbox-bug-address-select-1927258-3.patch queued for re-testing.

Version:6.x-2.x-dev» 7.x-1.x-dev
Status:Needs review» Fixed

Committed #3 to the 6.x-2.x branch.

Setting issue back to the version it was originally reported for and mark it as fixed.

Status:Fixed» Closed (fixed)

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