This is a follow-up from: #1293166-53: Whole node form is saved instead of widget form, possible data loss
I haven't had time to read through all the comments or related issues here. It seems clear to me that the ajax implementation in Hierarchical Select leaves a lot to be desired. It really needs to be refactored to pass in a specific _triggering_element value.
In our case, we're seeing a problem because we're using Admin Language and the default front-end language is Spanish while the default back-end language is English. So the node edit form is built in English when on node/*/edit, but the ajax requests hit the hierarchical_select_ajax path which is not considered an admin path. So on submission form builder rebuilds the form in Spanish. Thus the submitted form has different values as compared to the rebuilt form for any elements that have had their values translated. This causes the code in _form_button_was_clicked() to not be able to determine which form element made the submission.
So we're seeing a bug merely because the word "Update" was translated on our site.
For our use-case the Hierarchical Select widget is never going to be used on any non-admin pages so our band-aid fix is to use hook_admin_paths_alter() to mark hierarchical_select_ajax as an admin path to ensure it always gets built in the same language as the submitted form was built in.
Bandaid: Implement hook_admin_paths_alter.
Long-term: refactor the ajax implementation to pass in a specific _triggering_element value.
Refactor ajax
n/a
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | hierarchical_select-2410351-5.patch | 6.1 KB | stefan.r |
Comments
Comment #1
heddnComment #2
stefan.r commentedNote there is also #2378177: Admin theme not retained in AJAX requests
The band-aid would be problematic if the widget were used on non-admin pages. Is there a way for us to identify within hook_admin_paths_alter whether the AJAX request is being called from an admin path?
Comment #3
azinck commentedI'm the one who originally made that comment. The current 3.x-dev version does not resolve the problem. If you have the word "Update" translated on your site, and are using a different language for the front-end than the back-end, then you will face this problem. Drupal does not detect which button was pressed in that situation. Indeed, I suspect that there are a number of other edge cases (not language-related) that could cause the wrong button press to be detected if you have another button in the form with "Update" as its value. But the code there is a little confusing to me and I don't have a super strong handle on what all is happening.
The real solution here doesn't involve hook_admin_paths_alter. We shouldn't put more band-aids on top of Hierarchical Select's existing ajax solution. I think _triggering_element needs to be set so that we don't have to rely on Drupal guessing at which button was pressed but I need to take a closer look at this before I can submit a patch.
Comment #4
azinck commentedIt just occurred to me, though, that it's not just a matter of the button being translated which could cause problems; for other users who have translated entities they're going to get the wrong translations. So we really need to rebuild the form in the correct language. Frankly, I don't know the best way to do this. I assume that the "normal" ajax approach (involving ajax_form_callback()) handles this correctly somehow, but I don't know for sure.
Comment #5
stefan.r commentedhook_admin_paths() is not the right solution for admin_language either, as we'd need to change it based on whether we are "currently" on an admin path or not, and admin_language just saves the admin paths during a cron run into a variable and overrides the language during hook_init() based on that.
I agree porting the AJAX form elements to the D7 AJAX system would be the "proper" approach though, so patches would be most welcome!
For now, all we can do as a band-aid is make sure the language is the same between the page with the widget and the AJAX callback. Hence this convoluted patch :)
Comment #7
stefan.r commentedComment #8
stefan.r commented