site js using wrong jQuery replacement method [#1765546]

Hi,

I've noticed that the hosting_site.form.js is replacing field descriptions when a field has only one valid value with the text of the only value. This is fine until a hook_form_alter is altering the field reducing it to one value only. In this specific case the current code

$desc_id.show()
            .find('div.placeholder')
            .contents().
            replaceWith($.trim($input_id.val()));

when contents() method is call it returns an empty element. Replacing it like this example

$desc_id.show()
            .find('div.placeholder')
            .html($.trim($input_id.val()));

solves the problem.
Moreover contents() method (similar to children()) is used to fetch the element nodes of the given element. In this context is quite useless because we're trying to fetch nodes and then wipe them out with the replaceWith() call.

Comment	File	Size	Author
#1	hostmaster-wrong_jqeury_replacement_method-1765546-2.patch	1.38 KB	Bladedu

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Bladedu

he/him

Italian

Italy

CreditAttribution: Bladedu commented 31 August 2012 at 12:54

Status:

Active

» Needs review

File	Size
hostmaster-wrong_jqeury_replacement_method-1765546-2.patch	1.38 KB

Here's the patch that fixes the issue.

Comment #2

Steven Jones CreditAttribution: Steven Jones commented 31 August 2012 at 13:10

Would it be possible to provide a reproducible test case so we can test this fix please?

I.e. does this happen with a clean install of Aegir? If not, then could you provide a minimal form alter to reproduce this issue?

Comment #3

Bladedu

he/him

Italian

Italy

CreditAttribution: Bladedu commented 31 August 2012 at 13:30

Well I would change it anyway do to a misuses of jQuery contents() method, but you can easily reproduce the bug enabling SSL support and then implementing this in hook_form_alter for a site node form

if ($form_id == 'site_node_form') {
  $keys = hosting_ssl_get_keys();
  $form['ssl_key']['#options'] = $keys;
  // Set default value to the first value of the list.
  $form['ssl_key']['#default_value'] = array_shift(array_keys($keys));
}

and implement this in hook_hosting_site_options_alter()

if (isset($return['ssl_key']) && $node->ssl_enabled != 0) {
    $ssl_keys = hosting_ssl_get_keys();
    $return['ssl_key'] = array_keys($ssl_keys);
}

Comment #4

Steven Jones CreditAttribution: Steven Jones commented 31 August 2012 at 13:31

Thanks for the information, we'll get this fix in.

Comment #5

Steven Jones CreditAttribution: Steven Jones commented 14 September 2012 at 12:46

Status:

Needs review

» Postponed (maintainer needs more info)

Sorry, can you provide a screenshot of what you mean please? I'm struggling reproduce any issue and then apply a fix to it.

Comment #6

Bladedu

he/him

Italian

Italy

CreditAttribution: Bladedu commented 18 September 2012 at 13:43

I'm trying to create some screenshots to show you what's happening (I have to prepare everything again to reproduce the bug).
Meanwhile you can check about contents documentation (http://api.jquery.com/contents/)
and html(value) (http://api.jquery.com/html/#html2).

Apart from the bug encountered, I think it is better to use html() method because it's faster than calling contents() (which fetch the HTML of a given selector and then builds jquery objects) and the replacing everything that has been just parsed with replaceWith() method.