support hs_content_taxonomy widget [#741968]

When working with hierarchical_select, an error occurs.

PHP Fatal error: Cannot unset string offsets in .../comment_driven_cck.module on line 353

Please see 5 screenshots.

1. When not changing a term, an error occurs.
2. When changing a term, an error occurs.
3. When changing a term to <none>, an error doesn't occur.
4. HS settings
5. Term list

Comment	File	Size	Author
#13	2010-03-15_741968_support_hs_content_taxonomy_widget[unix].patch	8.52 KB	arhak
#11	1-1_single.png	58.53 KB	Takafumi
#11	1-2_single.png	60.97 KB	Takafumi
#11	1-3_single.png	64.17 KB	Takafumi
#11	2-1_multiple_with_dropbox.png	62.83 KB	Takafumi
#11	2-2_multiple_with_dropbox.png	62.35 KB	Takafumi
#11	2-3_multiple_with_dropbox.png	65.49 KB	Takafumi
#6	2010-03-14_741968_support_hs_content_taxonomy_widget[unix].patch	3.61 KB	arhak
	5_HS.png	52.61 KB	Takafumi
	4_HS.png	86.5 KB	Takafumi
	3_HS.png	61.17 KB	Takafumi
	2_HS.png	59.53 KB	Takafumi
	1_HS.png	61.19 KB	Takafumi

Comments

Comment #1

Takafumi commented 14 March 2010 at 11:01

FYI, an error occurs in both previewing and submitting.

Comment #2

arhak commented 14 March 2010 at 14:48

version numbers?
- CCK
- content_taxonomy
- HS

Comment #3

Takafumi commented 14 March 2010 at 15:02

It tested with the following versions:

- CCK
6.x-2.6 / latest dev

- content_taxonomy
6.x-1.0-rc2 / latest dev

- HS
6.x-3.0 / latest dev

Comment #4

Takafumi commented 14 March 2010 at 15:22

FYI, I grabbed some values as follows.

about line 291:

  // wrap separated item into items
  if ($separated_item) {
    $default = array($default);
    $value = array($value);
    drupal_set_message(t('line 294: $default = @default / $value = @value', array('@default' => var_export($default, true), '@value' => var_export($value, true))));
  }

about line 345:

  // unwrap separated item from items
  if ($separated_item) {
    // pick only the first item
    // since remaning items were filled with equivalent empty CCK values
    drupal_set_message(t('line 349: $default = @default / $value = @value', array('@default' => var_export($default, true), '@value' => var_export($value, true))));
    $default = array_shift($default);
    $value = array_shift($value);
    // dismiss non-value meta
    // '_weight' should come cleared from 'presave' (_content_sort_items)
    // but it won't harm to cover it here as well (just in case)
//    unset($value['_weight'], $value['_error_element']);

results:

    * line 294: $default = array ( 0 => array ( 0 => '1', ), ) / $value = array ( 0 => '1', )
    * line 349: $default = array ( 0 => array ( 'value' => NULL, ), ) / $value = array ( 0 => '1', )

Comment #5

Takafumi commented 14 March 2010 at 16:45

When term-2 is selected:

    * line 294: $default = array ( 0 => array ( 0 => '1', ), ) / $value = array ( 0 => '2', )
    * line 349: $default = array ( 0 => array ( 'value' => NULL, ), ) / $value = array ( 0 => '2', )

When "none" is selected:

    * line 294: $default = array ( 0 => array ( 0 => '1', ), ) / $value = array ( 0 => NULL, )
    * line 349: $default = array ( 0 => array ( 'value' => NULL, ), ) / $value = array ( 0 => array ( 'value' => NULL, ), )

Comment #6

arhak commented 14 March 2010 at 16:54

Title:	PHP Fatal error: Cannot unset string offsets	» support hs_content_taxonomy widget
Status:	Active	» Needs review

Status	File	Size
new	2010-03-14_741968_support_hs_content_taxonomy_widget[unix].patch	3.61 KB

the attached patch still has a pending limitation:
- if the vocabulary is multiple you should select save_lineage for hs_content_taxonomy
- otherwise you shouldn't select save_lineage nor dropbox
- i.e. the multiplicity of the actual taxonomy vocabulary has to match the multiplicity of hs_content_taxonomy

the multiplicity issue might be addressed as well, but after sorting this out first
and it is due to what hs_content_taxonomy states:

Important!

Due to limitations of CCK, there is a separate form to configure this Hierarchical Select widget's settings.

The Multiple values field setting is now managed by the Hierarchical Select module: it will be enabled when either the Enable the dropbox or Save term lineage settings (or both) are enabled.

also note that the attached patch intends to delegate content_taxomony widgets to comment_driven_taxo,
so whenever content_taxonomy widget issues arise they most likely will be reflected as being comment_driven_taxo issues
e.g. right now, violating the multiplicity match will display the following assertion:
Assertion failed: "$voc->multiple != HS save_lineage ..."
followed by two warning: array_diff() [function.array-diff]: Argument #X is not an array in .../comment_driven.eq.inc

we'll sort that out later,
note that it is due to hs_content_taxonomy handling multiplicity out of CCK's API

Comment #7

arhak commented 14 March 2010 at 17:07

@#3 thanks

@#4-5 I really appreciate your effort to provide detailed reports
the screenshots in the original post were very explicit (images, plus their descriptions)
but this dumped data was not, and I suggest you not to waste time trying to provide details around the lines which reflected the errors, since they aren't used to be what caused it
I don't like the idea of wasting anybody's time

Comment #8

arhak commented 14 March 2010 at 23:20

the proposal of patch @#6 is to delegate taxonomy widgets to comment_driven_taxo
but it seems to me that vanilla content_taxonomy uses same widgets than CCK, and therefore those shouldn't be delegated or CD_taxo shall fail

delegation should be then conditioned to the type of widget in use
currently unstable6 takes care of CCK's widgets (vanilla content_taxonomy)
but the patch proposed @#6 while addressing other kind of taxonomy widgets stops caring about CCK's widgets

Comment #9

Takafumi commented 15 March 2010 at 08:45

#6 patch works fine for me though it has some limitations.
However, I also still like using core of CCK's widgets and/or Multiselect widget.
So, would you please give an option so that these widgets can be used by trade-off?

Comment #10

arhak commented 15 March 2010 at 09:04

no trade-off is needed, everything can be supported (IF the proposed patch is working as expected/described)

I just made a quick patch for review, limitations are temporal,
since multiplicity issue of hs_content_taxonomy needs review,
but I trust its maintainer's judgement and code integrity,
it would be just a matter of making comment_driven support a different API (hs_content_taxonomy's multiplicity differing from vocabulary's multiplicity)

I also still like using core of CCK's widgets

when making the quick patch, I didn't realized that I was cutting off CCK's widgets, and delegated everything to comment_driven_taxo,
it would just a matter of merging both supports,
they are not incompatible at all

and/or Multiselect widget

I'm not sure what you meant
CCK's widget? hs_content_taxonomy widget?
with this patch applied you can use multiplicity of hs_content_taxonomy, the limitation is that it has to be the same multiplicity of the vocabulary (i.e. "multiple voc with multiple HS" or "single voc with single HS"), since there is an assertion guarding for that, which should be lessen (we'll get there)

in the meantime, please, be sure there won't be any surprise,
the proposed patch has to be right for the above statements to be truthful

Comment #11

Takafumi commented 15 March 2010 at 10:14

Status	File	Size
new	2-3_multiple_with_dropbox.png	65.49 KB
new	2-2_multiple_with_dropbox.png	62.35 KB
new	2-1_multiple_with_dropbox.png	62.83 KB
new	1-3_single.png	64.17 KB
new	1-2_single.png	60.97 KB
new	1-1_single.png	58.53 KB

Ok, it seems #6 patch works fine.
Please see some screenshots.

1-1. changes a single term
1-2. previewing it
1-3. submitted it

2-1. adds multiple term
2-2. previewing it
2-3. submitted it

Comment #12

arhak commented 15 March 2010 at 11:27

Status:

Needs review

» Needs work

@#11 very useful, thanks a lot
I'll be back to you soon

Comment #13

arhak commented 16 March 2010 at 07:26

Status:

Needs work

» Needs review

Status	File	Size
new	2010-03-15_741968_support_hs_content_taxonomy_widget[unix].patch	8.52 KB

now it should have no limitations

it should not break any vanilla content_taxonomy widgets:
- selects
- radios/checkboxes
- autocompletion

remember that hs_content_taxonomy is being delegated to comment_driven_taxo
which has to be considered when reporting issues that might show up as comment_driven_taxo issues

Comment #14

arhak commented 16 March 2010 at 08:39

Status:

Needs review

» Fixed

committed to HEAD

PS: HS dropbox hasn't been tested at all,
since it was being ignored (assertion mentioned at #6), now it is supported as well

Comment #15

Takafumi commented 16 March 2010 at 15:59

I tested on hierarchical_select widget, content_taxonomy's widgets, and Multiselect widget;

- hierarchical select
- autocomplete
- select list
- checkboxes/radios
- Multiselect

It seems these are now working properly with comment_driven.
Thanks a lot for your great work:)

Comment #16

arhak commented 16 March 2010 at 17:02

@#15 great news

BTW, the above dropbox note (#14) was valid when having dropbox without save_lineage
since save_lineage was properly supported at the time (obviously, as they were used for #11 screenshots)

Comment #17

BenK commented 17 March 2010 at 05:25

Keeping track of this thread...

Comment #18

31 March 2010 at 05:30

Status:

Fixed

» Closed (fixed)

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

support hs_content_taxonomy widget

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

News items

Our community

Documentation

Drupal code base

Governance of community