Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#30 | 1525152-29.patch | 717 bytes | dawehner |
#24 | views-format-key-1525152-24.patch | 3.16 KB | Berdir |
#22 | views-format-key-1525152-21.patch | 3.76 KB | Berdir |
#21 | views-format-key-1525152-21-test-only.patch | 2.54 KB | Berdir |
#21 | views-format-key-1525152-21-test-only.patch | 2.54 KB | Berdir |
Comments
Comment #1
dawehnerThis 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.
Comment #2
Georgique CreditAttribution: Georgique commentedI would be glad but just have not any experience in creating unit tests.
My be you would give me some direction?
Comment #3
dawehnerTake 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.
Comment #4
Georgique CreditAttribution: Georgique commentedDon't sure that I understood everything right, but check new patch =)
Comment #5
Georgique CreditAttribution: Georgique commentedThis issue seems not so helping you as just my studying ))
Comment #6
dawehnerUpdate status to trigger the testbot.
Comment #7
tim.plunkettWas this deletion intentional?
Trailing whitespace
Comment #8
Georgique CreditAttribution: Georgique commented@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?
Comment #9
Georgique CreditAttribution: Georgique commentedComment #11
dawehnerThe 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
Comment #12
asteroid.miner CreditAttribution: asteroid.miner commentedThis is still "in the works," yes?
I just ran into this error today:
Drush says Views is up to date:
Thanks, and good luck with it!
-BC
Comment #13
asteroid.miner CreditAttribution: asteroid.miner commentedYou'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
Comment #14
dawehnerLets reupload the patch and see what the testbot thinks about that.
Comment #15
rogical CreditAttribution: rogical commentedApplied 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).
Comment #16
dawehnerreuploaded 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.
Comment #17
myselfhimself CreditAttribution: myselfhimself commentedHello,
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 !
Comment #18
dawehnerCould 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.
Comment #19
calmforce CreditAttribution: calmforce commentedI 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).
Comment #20
dawehnerYeah 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.
Comment #21
BerdirWow, 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
Comment #22
Berdir... 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.
Comment #23
BerdirLooks like it also results in partially lost issue titles ;)
Comment #24
BerdirRemoving 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.
Comment #25
SeyV CreditAttribution: SeyV commentedPatch in #24 is working like a charm. Thanks!
Comment #26
dawehnerUps I committed this patch already on feb 19th :)
Comment #28
dawehnerNote: This would actually need a backport to 6.x-3.x as well.
Comment #30
dawehnerCommitted the following version of the patch.