I saw this error when installed PHP 5.4:
Warning: Illegal string offset 'format' in function views_object->unpack_translatable() (string 330 in file /home/george/www/inti.kz.dev/sites/all/modules/views/includes/base.inc).
Checked it and think that there should be checked not if $storage[$definition['format_key']] exists, but $options[$definition['format_key']].
Just take look into the patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Title: Invalid check in views_object::unpack_translatable() » Tests: format_key in unpack_options
Category: bug » task
Status: Needs review » Active

This makes absolute sense, committed to 7.x-3.x and 6.x-3.x

Regarding the error, php 5.4 seems to have changed the way isset() works internally, but hey this showed us a new bug.

It would be cool if this issue would have been covered by tests, so create a follow up.

Georgique’s picture

I would be glad but just have not any experience in creating unit tests.
My be you would give me some direction?

dawehner’s picture

Take a look at tests/views_translatable.test

Actually this shouldn't be too hard here:
Change the view which is defined in view_unpack_translatable() to include a header with a input format.
Then specify the translations in testTranslationKey() $this->string_keys to include the translation header text, this should be basically it.

Does this help you? Thanks for the will to work on this issue.

Georgique’s picture

FileSize
1.94 KB

Don't sure that I understood everything right, but check new patch =)

Georgique’s picture

This issue seems not so helping you as just my studying ))

dawehner’s picture

Status: Active » Needs review

Update status to trigger the testbot.

tim.plunkett’s picture

Status: Needs review » Needs work
+++ b/tests/views_translatable.testundefined
@@ -109,3 +109,2 @@ class ViewsTranslatableTest extends ViewsSqlTest {
-        'Master1' => array('title'),

Was this deletion intentional?

+++ b/tests/views_translatable.testundefined
@@ -131,3 +131,3 @@ class ViewsTranslatableTest extends ViewsSqlTest {
     $view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
-
+   ¶

Trailing whitespace

Georgique’s picture

FileSize
1.75 KB

@tim.plunkett Yes, I deleted it because there are two same strings 'Master1' => array('title').
Deleted trailing whitespace. New patch attached.

Just interesting, is other code written correct? When I add
'Header1' => array('header', 'area', 'content'),
what should be in those array?

Georgique’s picture

Status: Needs work » Needs review
dawehner’s picture

FileSize
2.56 KB
1.32 KB
'Header1' => array('header', 'area', 'content'),

The array is the "path" to the value stored in the view.

Did you tryed to run the test without the fix from above? This test should then fail ...
Let's reupload the patch, one with only the patch, and one which should actually fail, because the fix, which is already in, is reverted.

Let's hope the patch is tested this time

asteroid.miner’s picture

This is still "in the works," yes?

I just ran into this error today:

Warning: Illegal string offset 'format' [etc.]

Drush says Views is up to date:

Views (views)              7.x-3.3        7.x-3.3        Up to date   

Thanks, and good luck with it!

-BC

asteroid.miner’s picture

You're absolutely right, dawehner.

My post http://drupal.org/node/1666112
was indeed a duplicate of this one.
I'd forgotten I added a note here.

Is this issue the best place to watch for updates?

Thanks,

-BC

dawehner’s picture

FileSize
2.56 KB
1.32 KB

Lets reupload the patch and see what the testbot thinks about that.

rogical’s picture

Applied view_0_0.patch, error still exsits in my site.

Warning: Illegal string offset 'format' in views_object->unpack_translatable() (line 330 of /opt/development/pjkaixin/sites/all/modules/views/includes/base.inc).

dawehner’s picture

FileSize
1.32 KB
2.56 KB

reuploaded the patch to get the bot going.

@rogical: Only the second patch fixes the issue, the first one just adds a patch which makes sure that the test is writte in a proper way.

myselfhimself’s picture

Hello,

I still have that warning with Views 7.x-3.3.
It namely happens during drush fra

Could you please commit the patch for it ?

Take care !

dawehner’s picture

Could you please review the patch?

It's not just a matter of commiting a certain feature.

Btw. this will not be ported to d8 as the translation works differently.

calmforce’s picture

I believe this issue is the same that I came across in ver. 6.x-3.x-dev (see http://drupal.org/node/1863238 ) - incorrect usage of a string $options as an array, so you get "Illegal string offset 'format'".
The fix suggested here doesn't fix it because the array that holds the format value is actually $storage, not $options at line 330, so it has to be
$format = $storage[$definition['format_key']];

The $definition['format_key'] is 'format' and $storage['format'] = '3' in my situation (full html).

dawehner’s picture

Yeah i absolute agree that this is a duplicate: #1863238: 'format_key' bug in includes/base.inc
Reupload both patches so the testbot hopefully can test them, we certainly should have tests for that.

Berdir’s picture

Title: Tests: format_key in unpack_options » format_key handling broken which results in lost
Version: 7.x-3.3 » 7.x-3.x-dev
Category: task » bug
Priority: Normal » Major
FileSize
2.54 KB
2.54 KB

Wow, that was a fun ride ;)

This bug was introduced in #1167562-20: Undefined index: format in views_object->unpack_options() (line 143 of /sites/all/modules/views where it accidently switched from $storage to options while moving code around. (Hint: Those variable names are crazy ;)) But the isset() was still correctly using $storage.

The inital patch here then also changed the isset instead of fixing it the other way round.

Also updated the test and added test coverage for the format which the existing test did not yet do.

Note: Not sure why the key is now "header, views, area, content" (the views is new) but that seems to be there on real translations as well, so not changing that.

Changing to a major bug as the result of this bug is that translations get deleted because of the invalid format and there are long-standing bugs for this in i18nviews which I'm going to notify now:
- #1182210: translate views header / footer / Empty - Global: Text area in D7
- #1915130: Footer and header strings lose their translations on edit/save

Berdir’s picture

... Yeah, uploading the test only patch twice won't really fix the bug :)

Note 2: The fix also contains a related change for which I don't have test coverage, the change comes from @dawehner and looks good to me but I don't know under which contains that is triggered.

Berdir’s picture

Title: format_key handling broken which results in lost » format_key handling broken which results in lost translations

Looks like it also results in partially lost issue titles ;)

Berdir’s picture

Removing the first part that has no test coverage. I think it is currently broken and I think the original snippet that ·@dawehner gave me was the correct fix but what I added in the patch was even more broken than before.

So removing that and staying with the fix that is major and has test coverage. We could keep this open and change it to normal or even minor to investigate the other part.

SeyV’s picture

Patch in #24 is working like a charm. Thanks!

dawehner’s picture

Status: Needs review » Fixed

Ups I committed this patch already on feb 19th :)

Status: Fixed » Closed (fixed)

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

dawehner’s picture

Version: 7.x-3.x-dev » 6.x-3.x-dev
Issue summary: View changes
Status: Closed (fixed) » Patch (to be ported)

Note: This would actually need a backport to 6.x-3.x as well.

  • dawehner committed aa1deef on 6.x-3.x authored by Berdir
    Issue #1525152 by dawehner, Berdir, Georgique: format_key handling...
dawehner’s picture

Status: Patch (to be ported) » Fixed
FileSize
717 bytes

Committed the following version of the patch.

Status: Fixed » Closed (fixed)

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