Tried the dev version but got the "invalid response from server" error.

Basically the title says it all - when I click "Preview" when editing content, the HS values reset back to default. They're applied just fine if I set them again before saving the node.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Assigned: Unassigned » Wim Leers

The first problem should be fixed for you, or it could require this patch: #1160694: Received an Invalid Response from Server. Please start a new issue if neither of these fix the "Invalid response from server" error from you.

The title says it all indeed. Reproduced. Working on a solution.

Wim Leers’s picture

Title: "Preview" resets HS values » "Preview" resets HS values (needs patch/sponsoring)
Status: Active » Postponed

I give up. There's some very weird stuff going on in Field API/Forms API, or both.

Either way, I spent over 3 hours on this with zero progress. My entire Sunday forenoon. Without any results. This will need either a contributed patch or some sponsoring.

Wim Leers’s picture

Title: "Preview" resets HS values (needs patch/sponsoring) » "Preview" resets HS values

Also see this tweet plus my conversation with @fago: https://twitter.com/wimleers/status/105217706457968640

Wim Leers’s picture

Here are fago's relevant tweets:
- https://twitter.com/#!/the_real_fago/status/105224499468828672
- https://twitter.com/#!/the_real_fago/status/105225652420087808
- https://twitter.com/#!/the_real_fago/status/105226717324521472
- https://twitter.com/#!/the_real_fago/status/105228781291180032

The annoying thing is that his tweets are actually broken: they're not linked to the tweets they reply to, hence a conversation view is impossible. So it's actually quite hard to read this if you're not looking at this through my Twitter timeline.

Wim Leers’s picture

Here's an idea: sponsor fago to fix this :)

sun’s picture

I'm pretty sure you could save yourself from most headaches by dropping the custom hierarchical_select_ajax() entirely and switch to a native #ajax callback instead. Core's AJAX framework already takes over the required form and form_state massaging for you.

Wim Leers’s picture

AFAIK this is unrelated to HS' AJAX functionality, but it is strongly related to how #process works, when it is called, etc. Am I wrong here?

P.S.: thank you *very* much for providing your advice! :) It's much appreciated.

fago’s picture

I'm at the Drupalcon right now, followed by some days of visiting London - so I don't have much time right now :) though I'm happy to help with feedback / pointers where I can.

@#ajax: yep, if feasible that's definitely a good idea.

Wim Leers’s picture

Much appreciated, fago! :)

Let's talk about this after DrupalCon, then :)

michaelgiaimo’s picture

Appreciate you guys working on this. Truly remarkable module, no idea why Drupal wouldn't have something like this in core.

michaelgiaimo’s picture

Any progress on this?

Wim Leers’s picture

I'm afraid not yet, no.

michaelgiaimo’s picture

How do I go about sponsoring the fix?

Wim Leers’s picture

I don't have any time left for this, I'm afraid.

Riyuzakki’s picture

I fix it. Patch work for me. (7.x-3.0-alpha5)

RumpledElf’s picture

That patch didn't work on the dev version ...

skizzo’s picture

patch #15 applied to 7.x-3.0-alpha5 works for me. Note that I did not see the "invalid response from server" error, but still previewing resulted in the reset of HS values. Should the issue status be changed to "needs review"?

skizzo’s picture

One side-effect of patch #15 applied to 7.x-3.0-alpha5 is that whenever I add/edit a taxonomy term to any vocabulary I see the following notice
Notice: Undefined index: #field_name in form_type_hierarchical_select_value() (line 827 of /var/www/drupal/sites/all/modules/hierarchical_select/hierarchical_select.module).

ncbezi’s picture

Status: Postponed » Active
FileSize
2.38 KB

This patch fixed our problem and includes patch #15.

Please test it.

whitingx’s picture

The patch in #15 worked well for me.

Thanks very much for this @Riyuzakki.

chriszero’s picture

Version: 7.x-3.0-alpha2 » 7.x-3.0-alpha5

Patch in #19 is working for me.

Taxoman’s picture

Status: Active » Reviewed & tested by the community

Time to push it to -dev. With the current state/activity of this module, that will at least bring more feedback and confirmations.

Open Social’s picture

Patch from #19 works for me!

Nikit’s picture

Priority: Major » Normal
Status: Reviewed & tested by the community » Needs review

For patch #19 this code:

    if (!isset($form_state['storage']['hs']['last_hsid']) || isset($from_state['node_preview'])) {
      $form_state['storage']['hs']['last_hsid'] = -1;
      $form_state['storage']['hs']['is_node_preview'] = 1;
    }

should be replaced by next for supporting 2 or more term fields in node:

    if (!isset($form_state['storage']['hs']['last_hsid'])) {
      if (isset($from_state['node_preview'])) {
        $form_state['storage']['hs']['is_node_preview'] = 1;
      }
      $form_state['storage']['hs']['last_hsid'] = -1;      
    }

Otherwise other term fields with HS will disappear...

seaneffel’s picture

Priority: Normal » Critical
Status: Needs review » Active

I'm escalating this issue because it both prevents users from completing tasks and it deletes users original data. The term reference fields are wiped after preview on both new nodes and old nodes. Sites with a moderation queue are likely to have moderators accidentally wiping users original term data without any warning or notice (unless the term ref fields are required).

The patch in #19 corrects the problem if there is only one term reference field using hierarchical select widget. But if there are two or more term reference fields using hierarchical select, then the second field again has its values wiped. #24 tries but does not fix it.

This issue has been open for 11 month!

seaneffel’s picture

Title: "Preview" resets HS values » Previewing nodes deletes term reference values
viggo’s picture

#19 works for me. I have only one hs field.

peterx’s picture

I applied patch in 19 and it fixed the problem with our single select in the form.
The patch also created the problem in 18 but at line 832.
I changed the if() to:
if (isset($element['#field_name']) and isset($form_state['values'][$element['#field_name']])) {

spcalpo’s picture

Version: 7.x-3.0-alpha5 » 7.x-3.x-dev

I just wanted to reiterate that applying patch in #19, then manually editing to add changes from #24 (for multiple HS terms) and #28 (fix #field_name error) fixed this preview issue for me.

I am at work so no time to generate a new patch for this right at this moment, but hoping my experience may help the process of committing this fix. It's a useful one.

seaneffel’s picture

Oh boy, but it would be awesome if you could roll out a patch some time. Some of us ^^ have no ability to do that for ourselves!

acrollet’s picture

Status: Active » Needs review
FileSize
2.15 KB

here's a re-roll of #19 incorporating the suggestions in #24 and #28.

Sharique’s picture

Hi acrollet,
I did tried tried your patch, but it is not working for me. I tried it 3.x branch.

hanoii’s picture

Status: Needs review » Needs work

Patch in #31 doesn't work, I have serveral hierarchical select fields and once you preview them, although the values are not removed, if you change any hierarchical select fields that needs to update the hierarchy it removes every hierarchical select but one!

hanoii’s picture

I spent a considerable amount of time today trying to sort this and I think finally fixed it in a nice way.

Attached is a patch. This approach is completely different from the ones mentioned in this thread so it's a brand new patch, very easy to review as it only adds one condition to check if the preview button was clicked. The function that is prevented to be run from this check was clearing the data from the $form_state['input'] and that was wiping off the values for every element.

I don't think it's a bullet proof fix because the problem will still be there if the form is rebuilt by some other action, but for node's preview, this is a good enough fix.

I left this thought on the code as well:

  // @TODO: If the form is rebuilt by some other action than a node preview, we
  // might lose data again, we should see if there's any way to prevent this from
  // happening by setting this value after the form has been flagged to be rebuilt,
  // but as far as I checked, there's not.
  // Another option might be to rework the need of this function to prevent
  // the undesired behaviors of not having it with some other logic that would
  // work as well if the form is rebuilt.
hanoii’s picture

Status: Needs work » Closed (fixed)
acrollet’s picture

Status: Closed (fixed) » Needs review
hanoii’s picture

Right sorry, missed the proper state, needs review is what I was going for. Thanks

Sharique’s picture

Thanks hanoii,
This works for me. Although this patch failed to apply, I've to manually do the change.

hanoii’s picture

Patch is against the 7.x-3.x-dev, it fails for 7.x-3.0-alpha5, I recommend using -dev anyway, if you looked at the commits there are a few bugfixes that better having them than not, and considering this module is not very active lately, alpha should be as good as dev.

zet’s picture

hanoii , strange, but it looks like there are some issues with some whitespaces on your patchfile ?

[root@nemesis hierarchical_select]#  /usr/bin/patch -p1 --verbose -d '/var/www/html/drupal7/sites/all/modules/hierarchical_select' -i '/var/www/html/drupal7/sites/dev.****.ro/files/patches/1242812-34-hierarchical_select-preview-fix.patch'
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/hierarchical_select.module b/hierarchical_select.module
|index 731267d..5922910 100644
|--- a/hierarchical_select.module
|+++ b/hierarchical_select.module
--------------------------
Patching file hierarchical_select.module using Plan A...
Hunk #1 FAILED at 807.
1 out of 1 hunk FAILED -- saving rejects to file hierarchical_select.module.rej
done
 

patch is applied only using "--ignore-whitespace " :

[root@nemesis hierarchical_select]#  /usr/bin/patch -p1 --verbose -d '/var/www/html/drupal7/sites/all/modules/hierarchical_select' -i '/var/www/html/drupal7/sites/dev.****.ro/files/patches/1242812-34-hierarchical_select-preview-fix.patch' --ignore-whitespace
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/hierarchical_select.module b/hierarchical_select.module
|index 731267d..5922910 100644
|--- a/hierarchical_select.module
|+++ b/hierarchical_select.module
--------------------------
Patching file hierarchical_select.module using Plan A...
Hunk #1 succeeded at 807.
done
 

But once applied, I can confirm the node preview work as supposed, without loosing term reference values.
Thanks

hanoii’s picture

@zen, it's a simple enough patch created with git diff -w (to ignore whitespace), have you tried to apply it against the dev version?

zet’s picture

Yes hanoii, it's applied against latest dev version. Maybe that -w it's the problem. If you edited the file on windows u should use something like dos2unix command against the edited file, before making a diff patch.

hanoii’s picture

aha, well not on windows. I checked and you were right, something was off with the patch, attached is a tested one against latest codebase. This one works for alpha5 as well.

Aljullu’s picture

Patch #43 works for me. Thank you!

richsky’s picture

Issue summary: View changes

Well, what about revision and the view changes button (seems to act as the preview button)?

Adding

|| $form_state['triggering_element']['#value'] != t('View changes')

Doesn't do the trick...

but

&& $form_state['triggering_element']['#value'] == t('View Changes')

Does.

I guess in case of View Changes, the last submitted values apply.

WIStudent’s picture

Patch #43 works for me too. I applied it to the current 7.x-3.x branch.

jessehs’s picture

This patch adds the "View changes" button as one that prevents a rebuild. This button is provided by the Diff module.

Edit: Patch at the top of the issue in the "files" section. It's not actually attached to this comment, although it's named with this comment number (47).
Edit 2: oh, well it is attached now. Funny.

stefan.r’s picture

Status: Needs review » Needs work

This patch needs a re-roll against HEAD

stefan.r’s picture

Status: Needs work » Needs review
FileSize
1.57 KB

  • stefan.r committed e056d38 on 7.x-3.x
    Issue #1242812 by hanoii, acrollet, stefan.r, jessehs, ncbezi, Riyuzakki...
stefan.r’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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