I've added the google_appliance module to an installation profile. When the installation profile finishes and I click through to see the newly finished homepage, I'm presented with the following error:

Notice: Uninitialized string offset: 0 in google_appliance_block_view_alter() (line 310 of /sites/all/modules/contrib/google_appliance/google_appliance.module).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpgeek’s picture

Assigned: Unassigned » mpgeek

Merging issues here.

I've installed GSA and 3 red Notices at the top of the page greeted me eagerly saying that "Notice: Uninitialized string offset: 0 in google_appliance_block_view_alter() (string 310 in file \www\sites\all\modules\google_appliance\google_appliance.module). This was the culprit:

if (isset($settings['block_visibility_settings'][$module][$delta]))

Turned out $settings['block_visibility_settings'][$module] was NULL itself in my case and couldn't produce any [$delta]. I've changed this piece of code to:

if (isset($settings['block_visibility_settings'][$module]) && isset($settings['block_visibility_settings'][$module][$delta]))

and the Notices disappeared. It's good to have developers look at this bug though.

That's from #1700536: Notice: Uninitialized string offset: 0 in google_appliance_block_view_alter() appears on every page after installing the module ... It turns out that using an empty string for a default settings value that is meant to be an empty array at the start is causing problem. Patch coming soon.

mpgeek’s picture

I wasn't able to reproduce the error in any of my dev environments, but i did notice that after installation the array key's value is an empty string (instead of an empty array), and depending upon your PHP version isset() behaves a bit differently (note the "isset() on String Offsets") here: http://us2.php.net/manual/en/function.isset.php).

Anyways, a patch is attached that should fix that.

mpgeek’s picture

Status: Active » Needs review

Committed to dev: http://drupalcode.org/project/google_appliance.git/commit/3e8f763.

@Damien @Ari, do either of you have time to try this out and post back here whether or not this fixes issues you experienced? Thanks.

iamEAP’s picture

Hmm. Thought I'd covered this case in my patch, but I guess not. The variable handling code this module uses is pretty clever, but its weakness is in handling non-scaler variable data. Maybe eventually, we'll want to move away from storing these block visibility settings in a variable and throwing them into their own table (or appending them to the the block table), but the above patch looks like the best compromise for now.

Maybe the
if ($settings['block_visibility_settings'] == '')
should be....
if ($settings['block_visibility_settings'] === '')

mpgeek’s picture

I only noticed that in fact isset() was trying to index into a string upon fresh install, but it still worked in my environments. Going to wait for reports from Damien and Ari to see if it actually fixes it, as I cannot reproduce the error.

DamienMcKenna’s picture

Status: Needs review » Reviewed & tested by the community

The current -dev codebase no longer has the error.

mpgeek’s picture

Assigned: mpgeek » Unassigned
Status: Reviewed & tested by the community » Closed (fixed)

@DamienMcKenna, thanks for the feedback. Stable 7.x-1.8 has been pushed: http://drupal.org/node/1703058.