From 4367e9641356b13764aceb289c46b6023c6c6bb8 Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Sat, 20 Jun 2020 13:14:42 -0400 Subject: [PATCH] Attributes for a form element should always be an array --- CRM/Admin/Form/Job.php | 2 +- CRM/Admin/Form/MessageTemplates.php | 4 +- CRM/Admin/Form/PdfFormats.php | 2 +- CRM/Contact/Form/Task/EmailTrait.php | 2 +- .../Form/ContributionPage/Premium.php | 2 +- CRM/Contribute/Form/ManagePremiums.php | 6 +- CRM/Core/BAO/CustomField.php | 143 +++++++++--------- CRM/Core/Form.php | 11 +- CRM/Import/DataSource/SQL.php | 2 +- CRM/Price/Form/Field.php | 2 +- CRM/SMS/Form/Provider.php | 2 +- 11 files changed, 91 insertions(+), 87 deletions(-) diff --git a/CRM/Admin/Form/Job.php b/CRM/Admin/Form/Job.php index dec88aeeab..2738c1b49c 100644 --- a/CRM/Admin/Form/Job.php +++ b/CRM/Admin/Form/Job.php @@ -101,7 +101,7 @@ class CRM_Admin_Form_Job extends CRM_Admin_Form { $this->add('datepicker', 'scheduled_run_date', ts('Scheduled Run Date'), NULL, FALSE, ['minDate' => date('Y-m-d')]); $this->add('textarea', 'parameters', ts('Command parameters'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); // is this job active ? diff --git a/CRM/Admin/Form/MessageTemplates.php b/CRM/Admin/Form/MessageTemplates.php index 5f809f25be..47567b4742 100644 --- a/CRM/Admin/Form/MessageTemplates.php +++ b/CRM/Admin/Form/MessageTemplates.php @@ -179,7 +179,7 @@ class CRM_Admin_Form_MessageTemplates extends CRM_Core_Form { ) ) { $this->add('textarea', 'msg_html', ts('HTML Message'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); } else { @@ -194,7 +194,7 @@ class CRM_Admin_Form_MessageTemplates extends CRM_Core_Form { } $this->add('textarea', 'msg_text', ts('Text Message'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); $this->add('select', 'pdf_format_id', ts('PDF Page Format'), diff --git a/CRM/Admin/Form/PdfFormats.php b/CRM/Admin/Form/PdfFormats.php index 0ec9042d58..1f7c10a0f5 100644 --- a/CRM/Admin/Form/PdfFormats.php +++ b/CRM/Admin/Form/PdfFormats.php @@ -67,7 +67,7 @@ class CRM_Admin_Form_PdfFormats extends CRM_Admin_Form { ['onChange' => "selectPaper( this.value );"] ); - $this->add('static', 'paper_dimensions', NULL, ts('Width x Height')); + $this->add('static', 'paper_dimensions', ts('Width x Height')); $this->add('select', 'orientation', ts('Orientation'), CRM_Core_BAO_PdfFormat::getPageOrientations(), FALSE, ['onChange' => "updatePaperDimensions();"] ); diff --git a/CRM/Contact/Form/Task/EmailTrait.php b/CRM/Contact/Form/Task/EmailTrait.php index c426d8d4bb..67c06172d8 100644 --- a/CRM/Contact/Form/Task/EmailTrait.php +++ b/CRM/Contact/Form/Task/EmailTrait.php @@ -247,7 +247,7 @@ trait CRM_Contact_Form_Task_EmailTrait { $this->assign('totalSelectedContacts', count($this->_contactIds)); - $this->add('text', 'subject', ts('Subject'), 'size=50 maxlength=254', TRUE); + $this->add('text', 'subject', ts('Subject'), ['size' => 50, 'maxlength' => 254], TRUE); $this->add('select', 'from_email_address', ts('From'), $this->_fromEmails, TRUE); diff --git a/CRM/Contribute/Form/ContributionPage/Premium.php b/CRM/Contribute/Form/ContributionPage/Premium.php index ba5b7863b7..7683f3d900 100644 --- a/CRM/Contribute/Form/ContributionPage/Premium.php +++ b/CRM/Contribute/Form/ContributionPage/Premium.php @@ -52,7 +52,7 @@ class CRM_Contribute_Form_ContributionPage_Premium extends CRM_Contribute_Form_C $this->addElement('text', 'premiums_intro_title', ts('Title'), $attributes['premiums_intro_title']); - $this->add('textarea', 'premiums_intro_text', ts('Introductory Message'), 'rows=5, cols=50'); + $this->add('textarea', 'premiums_intro_text', ts('Introductory Message'), ['cols' => 50, 'rows' => 5]); $this->add('text', 'premiums_contact_email', ts('Contact Email') . ' ', $attributes['premiums_contact_email']); diff --git a/CRM/Contribute/Form/ManagePremiums.php b/CRM/Contribute/Form/ManagePremiums.php index 61da18d8f4..3a4a97e0ec 100644 --- a/CRM/Contribute/Form/ManagePremiums.php +++ b/CRM/Contribute/Form/ManagePremiums.php @@ -81,7 +81,7 @@ class CRM_Contribute_Form_ManagePremiums extends CRM_Contribute_Form { ]); $this->add('text', 'sku', ts('SKU'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'sku')); - $this->add('textarea', 'description', ts('Description'), 'rows=3, cols=60'); + $this->add('textarea', 'description', ts('Description'), ['cols' => 60, 'rows' => 3]); $image['image'] = $this->createElement('radio', NULL, NULL, ts('Upload from my computer'), 'image', 'onclick="add_upload_file_block(\'image\');'); $image['thumbnail'] = $this->createElement('radio', NULL, NULL, ts('Display image and thumbnail from these locations on the web:'), 'thumbnail', 'onclick="add_upload_file_block(\'thumbnail\');'); @@ -94,7 +94,7 @@ class CRM_Contribute_Form_ManagePremiums extends CRM_Contribute_Form { $this->addElement('text', 'imageUrl', ts('Image URL')); $this->addElement('text', 'thumbnailUrl', ts('Thumbnail URL')); - $this->add('file', 'uploadFile', ts('Image File Name'), 'onChange="select_option();"'); + $this->add('file', 'uploadFile', ts('Image File Name'), ['onChange' => 'select_option();']); $this->add('text', 'price', ts('Market Value'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'price'), TRUE); $this->addRule('price', ts('Please enter the Market Value for this product.'), 'money'); @@ -105,7 +105,7 @@ class CRM_Contribute_Form_ManagePremiums extends CRM_Contribute_Form { $this->add('text', 'min_contribution', ts('Minimum Contribution Amount'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'min_contribution'), TRUE); $this->addRule('min_contribution', ts('Please enter a monetary value for the Minimum Contribution Amount.'), 'money'); - $this->add('textarea', 'options', ts('Options'), 'rows=3, cols=60'); + $this->add('textarea', 'options', ts('Options'), ['cols' => 60, 'rows' => 3]); $this->add('select', 'period_type', ts('Period Type'), [ '' => '- select -', diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 41cc7fe7ba..030c88b17c 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -667,11 +667,13 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { $element = NULL; $customFieldAttributes = []; + // DAO stores attributes as a string, but it's hard to manipulate and + // CRM_Core_Form::add() wants them as an array. + $fieldAttributes = self::attributesFromString($field->attributes); + // Custom field HTML should indicate group+field name $groupName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id); - $dataCrmCustomVal = $groupName . ':' . $field->name; - $dataCrmCustomAttr = 'data-crm-custom="' . $dataCrmCustomVal . '"'; - $field->attributes .= $dataCrmCustomAttr; + $fieldAttributes['data-crm-custom'] = $groupName . ':' . $field->name; // Fixed for Issue CRM-2183 if ($widget == 'TextArea' && $search) { @@ -680,39 +682,34 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { $placeholder = $search ? ts('- any -') : ($useRequired ? ts('- select -') : ts('- none -')); - $isSelect = (in_array($widget, [ + if (in_array($widget, [ 'Select', 'CheckBox', 'Radio', - ])); - - if ($isSelect) { + ])) { $options = $field->getOptions($search ? 'search' : 'create'); // Consolidate widget types to simplify the below switch statement - if ($search || (strpos($widget, 'Select') !== FALSE)) { + if ($search) { $widget = 'Select'; } - $customFieldAttributes['data-crm-custom'] = $dataCrmCustomVal; - $selectAttributes = ['class' => 'crm-select2']; - // Search field is always multi-select if ($search || (self::isSerialized($field) && $widget === 'Select')) { - $selectAttributes['class'] .= ' huge'; - $selectAttributes['multiple'] = 'multiple'; - $selectAttributes['placeholder'] = $placeholder; + $fieldAttributes['class'] .= ltrim($fieldAttributes['class'] ?? '' . ' huge'); + $fieldAttributes['multiple'] = 'multiple'; + $fieldAttributes['placeholder'] = $placeholder; } // Add data for popup link. Normally this is handled by CRM_Core_Form->addSelect $canEditOptions = CRM_Core_Permission::check('administer CiviCRM'); - if ($field->option_group_id && !$search && $isSelect && $canEditOptions) { + if ($field->option_group_id && !$search && $canEditOptions) { $customFieldAttributes += [ 'data-api-entity' => $field->getEntity(), 'data-api-field' => 'custom_' . $field->id, 'data-option-edit-path' => 'civicrm/admin/options/' . CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', $field->option_group_id), ]; - $selectAttributes += $customFieldAttributes; + $fieldAttributes = array_merge($fieldAttributes, $customFieldAttributes); } } @@ -728,50 +725,39 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { case 'Text': case 'Link': if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { if ($field->text_length) { - $field->attributes .= ' maxlength=' . $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; if ($field->text_length < 20) { - $field->attributes .= ' size=' . $field->text_length; + $fieldAttributes['size'] = $field->text_length; } } $element = $qf->add('text', $elementName, $label, - $field->attributes, + $fieldAttributes, $useRequired && !$search ); } break; case 'TextArea': - $attributes = $dataCrmCustomAttr; - if ($field->note_rows) { - $attributes .= 'rows=' . $field->note_rows; - } - else { - $attributes .= 'rows=4'; - } - if ($field->note_columns) { - $attributes .= ' cols=' . $field->note_columns; - } - else { - $attributes .= ' cols=60'; - } + $fieldAttributes['rows'] = $field->note_rows ?? 4; + $fieldAttributes['cols'] = $field->note_columns ?? 60; + if ($field->text_length) { - $attributes .= ' maxlength=' . $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; } $element = $qf->add('textarea', $elementName, $label, - $attributes, + $fieldAttributes, $useRequired && !$search ); break; case 'Select Date': - $attr = ['data-crm-custom' => $dataCrmCustomVal]; //CRM-18379: Fix for date range of 'Select Date' custom field when include in profile. $minYear = isset($field->start_date_years) ? (date('Y') - $field->start_date_years) : NULL; $maxYear = isset($field->end_date_years) ? (date('Y') + $field->end_date_years) : NULL; @@ -784,40 +770,40 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { 'time' => $field->time_format ? $field->time_format * 12 : FALSE, ]; if ($field->is_search_range && $search) { - $qf->add('datepicker', $elementName . '_from', $label, $attr + array('placeholder' => ts('From')), FALSE, $params); - $qf->add('datepicker', $elementName . '_to', NULL, $attr + array('placeholder' => ts('To')), FALSE, $params); + $qf->add('datepicker', $elementName . '_from', $label, $fieldAttributes + array('placeholder' => ts('From')), FALSE, $params); + $qf->add('datepicker', $elementName . '_to', NULL, $fieldAttributes + array('placeholder' => ts('To')), FALSE, $params); } else { - $element = $qf->add('datepicker', $elementName, $label, $attr, $useRequired && !$search, $params); + $element = $qf->add('datepicker', $elementName, $label, $fieldAttributes, $useRequired && !$search, $params); } break; case 'Radio': if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { - parse_str($field->attributes, $radioAttributes); - $radioAttributes = array_merge($radioAttributes, $customFieldAttributes); + $fieldAttributes = array_merge($fieldAttributes, $customFieldAttributes); if ($search || empty($useRequired)) { - $radioAttributes['allowClear'] = TRUE; + $fieldAttributes['allowClear'] = TRUE; } - $qf->addRadio($elementName, $label, $options, $radioAttributes, NULL, $useRequired); + $qf->addRadio($elementName, $label, $options, $fieldAttributes, NULL, $useRequired); } break; // For all select elements case 'Select': + $fieldAttributes['class'] .= ltrim($fieldAttributes['class'] ?? '' . ' crm-select2'); if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { - if (empty($selectAttributes['multiple'])) { + if (empty($fieldAttributes['multiple'])) { $options = ['' => $placeholder] + $options; } - $element = $qf->add('select', $elementName, $label, $options, $useRequired && !$search, $selectAttributes); + $element = $qf->add('select', $elementName, $label, $options, $useRequired && !$search, $fieldAttributes); // Add and/or option for fields that store multiple values if ($search && self::isSerialized($field)) { @@ -836,6 +822,9 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { case 'CheckBox': $check = []; foreach ($options as $v => $l) { + // TODO: I'm not sure if this is supposed to exclude whatever might be + // in $field->attributes (available in array format as + // $fieldAttributes). Leaving as-is for now. $check[] = &$qf->addElement('advcheckbox', $v, NULL, $l, $customFieldAttributes); } @@ -859,44 +848,31 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { strtolower($field->html_type), $elementName, $label, - $field->attributes, + $fieldAttributes, $useRequired && !$search ); $qf->addUploadElement($elementName); break; case 'RichTextEditor': - $attributes = [ - 'rows' => $field->note_rows, - 'cols' => $field->note_columns, - 'data-crm-custom' => $dataCrmCustomVal, - ]; + $fieldAttributes['rows'] = $field->note_rows; + $fieldAttributes['cols'] = $field->note_columns; if ($field->text_length) { - $attributes['maxlength'] = $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; } - $element = $qf->add('wysiwyg', $elementName, $label, $attributes, $useRequired && !$search); + $element = $qf->add('wysiwyg', $elementName, $label, $fieldAttributes, $useRequired && !$search); break; case 'Autocomplete-Select': static $customUrls = []; - // Fixme: why is this a string in the first place?? - $attributes = []; - if ($field->attributes) { - foreach (explode(' ', $field->attributes) as $at) { - if (strpos($at, '=')) { - list($k, $v) = explode('=', $at); - $attributes[$k] = trim($v, ' "'); - } - } - } if ($field->data_type == 'ContactReference') { // break if contact does not have permission to access ContactReference if (!CRM_Core_Permission::check('access contact reference fields')) { break; } - $attributes['class'] = (isset($attributes['class']) ? $attributes['class'] . ' ' : '') . 'crm-form-contact-reference huge'; - $attributes['data-api-entity'] = 'Contact'; - $element = $qf->add('text', $elementName, $label, $attributes, $useRequired && !$search); + $fieldAttributes['class'] = ltrim($fieldAttributes['class'] ?? '' . ' crm-form-contact-reference huge'); + $fieldAttributes['data-api-entity'] = 'Contact'; + $element = $qf->add('text', $elementName, $label, $fieldAttributes, $useRequired && !$search); $urlParams = "context=customfield&id={$field->id}"; $idOfelement = $elementName; @@ -915,7 +891,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { } else { // FIXME: This won't work with customFieldOptions hook - $attributes += [ + $fieldAttributes += [ 'entity' => 'OptionValue', 'placeholder' => $placeholder, 'multiple' => $search, @@ -923,7 +899,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { 'params' => ['option_group_id' => $field->option_group_id, 'is_active' => 1], ], ]; - $element = $qf->addEntityRef($elementName, $label, $attributes, $useRequired && !$search); + $element = $qf->addEntityRef($elementName, $label, $fieldAttributes, $useRequired && !$search); } $qf->assign('customUrls', $customUrls); @@ -973,6 +949,27 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { return $element; } + /** + * Take a string of HTML element attributes and turn it into an associative + * array. + * + * @param string $attrString + * The attributes as a string, e.g. `rows=3 cols=40`. + * + * @return array + * The attributes as an array, e.g. `['rows' => 3, 'cols' => 40]`. + */ + public static function attributesFromString($attrString) { + $attributes = []; + foreach (explode(' ', $attrString) as $at) { + if (strpos($at, '=')) { + list($k, $v) = explode('=', $at); + $attributes[$k] = trim($v, ' "'); + } + } + return $attributes; + } + /** * Delete the Custom Field. * diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index fe06bb9915..f149bdecf3 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -350,7 +350,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page { * @param string $type * @param string $name * @param string $label - * @param string|array $attributes (options for select elements) + * @param array $attributes (options for select elements) * @param bool $required * @param array $extra * (attributes for select elements). @@ -364,11 +364,18 @@ class CRM_Core_Form extends HTML_QuickForm_Page { */ public function &add( $type, $name, $label = '', - $attributes = '', $required = FALSE, $extra = NULL + $attributes = NULL, $required = FALSE, $extra = NULL ) { if ($type === 'radio') { CRM_Core_Error::deprecatedFunctionWarning('CRM_Core_Form::addRadio'); } + + if ($attributes && !is_array($attributes)) { + // The $attributes param used to allow for strings and would default to an + // empty string. However, now that the variable is heavily manipulated, + // we should expect it to always be an array. + Civi::log()->warning('Attributes passed to CRM_Core_Form::add() are not an array.', ['civi.tag' => 'deprecated']); + } // Fudge some extra types that quickform doesn't support $inputType = $type; if ($type == 'wysiwyg' || in_array($type, self::$html5Types)) { diff --git a/CRM/Import/DataSource/SQL.php b/CRM/Import/DataSource/SQL.php index 26d3e09faa..c1cbb61e1e 100644 --- a/CRM/Import/DataSource/SQL.php +++ b/CRM/Import/DataSource/SQL.php @@ -49,7 +49,7 @@ class CRM_Import_DataSource_SQL extends CRM_Import_DataSource { */ public function buildQuickForm(&$form) { $form->add('hidden', 'hidden_dataSource', 'CRM_Import_DataSource_SQL'); - $form->add('textarea', 'sqlQuery', ts('Specify SQL Query'), 'rows=10 cols=45', TRUE); + $form->add('textarea', 'sqlQuery', ts('Specify SQL Query'), ['rows' => 10, 'cols' => 45], TRUE); $form->addFormRule(['CRM_Import_DataSource_SQL', 'formRule'], $form); } diff --git a/CRM/Price/Form/Field.php b/CRM/Price/Form/Field.php index a7f7420f9a..96aaf3b558 100644 --- a/CRM/Price/Form/Field.php +++ b/CRM/Price/Form/Field.php @@ -181,7 +181,7 @@ class CRM_Price_Form_Field extends CRM_Core_Form { $this->add('text', 'label', ts('Field Label'), CRM_Core_DAO::getAttribute('CRM_Price_DAO_PriceField', 'label'), TRUE); // html_type - $javascript = 'onchange="option_html_type(this.form)";'; + $javascript = ['onchange' => 'option_html_type(this.form);']; $htmlTypes = CRM_Price_BAO_PriceField::htmlTypes(); diff --git a/CRM/SMS/Form/Provider.php b/CRM/SMS/Form/Provider.php index 68e64de811..c78f55fcd4 100644 --- a/CRM/SMS/Form/Provider.php +++ b/CRM/SMS/Form/Provider.php @@ -94,7 +94,7 @@ class CRM_SMS_Form_Provider extends CRM_Core_Form { $this->add('text', 'api_url', ts('API Url'), $attributes['api_url'], TRUE); $this->add('textarea', 'api_params', ts('API Parameters'), - "cols=50 rows=6", TRUE + ['cols' => 50, 'rows' => 6], TRUE ); $this->add('checkbox', 'is_active', ts('Is this provider active?')); -- 2.25.1