Since we require PHP 5.2, we can rely instead on the built-in function json_encode()

http://www.php.net/manual/en/function.json-encode.php

Someone who knows more about the AHAH functionality should look at this- - also this change seem to break upload module's attach button, which suggests that there is some difference between the output of drupal_to_js() and json_encode()

CommentFileSizeAuthor
#2 improve-drupal_to_js-222578-2.patch1.82 KBpwolanin
remove-drupal_to_js.patch5.19 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

Ahh - investigation reveals that json_encode() does not escape < > or & the way drupal_to_js() does - thus there are HTML errors when the AHAH tries to get the updated form.

pwolanin’s picture

pwolanin commented
Title: remove drupal_to_js() » alter drupal_to_js() to use json_encode()
Status: Needs work » Needs review
StatusFileSize
new improve-drupal_to_js-222578-2.patch1.82 KB

new patch - leave drupal_to_js() so we can escape to HTML-safe, but use the built-in json_encode() for efficiency.

With this the upload form AHAH, at least, seems to work right.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Subscribing. Looking good! :)

pwolanin’s picture

pwolanin commented

tested with PHP 5.2.4, Mac 10.4, FF 2.0 and Safari 3.0, apache 1.3:

book, upload, poll module AHA all works fine, user autocomplete field in node form works fine.

joshk’s picture

joshk commented
Status: Needs review » Reviewed & tested by the community

Tested also in ie6 and ie7. Poll, book, upload and autocomplete all work.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

I committed this to CVS HEAD. Thanks.

kkaefer’s picture

kkaefer commented
Status: Fixed » Active

Note: I asked Steven about why he introduced the escaping in the first place, since everything seems to work fine without. The reason he put that into place is because it was required for the upload.module's iframe mechanism. This however, has been removed (and replaced with AHAH). That means, that we can get rid of the encoding after we tested that everything still works.

pwolanin’s picture

pwolanin commented

@kk - during my testing it did NOT work when the escaping was removed. See the first patch.

pwolanin’s picture

pwolanin commented
Status: Active » Fixed

back to fixed

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented
Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Patch (to be ported)

Does this need backporting to d6 (in order to help with upgrade to jQuery 1.4.x)?

PS: Changed status in order to gain some attention - don't mean to bust anyone's b@lls here. Feel free to change back if it is irrelevant, but it does seem to solve this #818138: jQuery 1.4.x compatibility in Hierarchical Select.

pwolanin’s picture

pwolanin commented
Version: 6.x-dev » 7.x-dev
Status: Patch (to be ported) » Closed (fixed)

I think this is closed in terms of core, certainly for Drupal 6.x.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Care to explain why Peter? Is it because d7 requires php 5.2 (that has json_encode() built-in) while d6 requires 4.4+ instead? If so, I think there could be a check to detect php version and then if >5.2 use json_encode(), if <5.2 use drupal_to_js().