Admin config form shows incorrect version for "Default jQuery version" when custom jQuery version is set but not saved as default

Created on 1 March 2023, almost 2 years ago

Problem/Motivation

When a value is saved for the "Custom jQuery version" variable (jquery_update_custom_version_jquery) but that value is not saved for "Default jQuery version" (jquery_update_jquery_version), the custom jQuery version nonetheless shows as the default (selected) value for "Default jQuery version" on the primary jQuery Update configuration form.

Steps to reproduce

  • Install the latest version of jQuery Update.
  • Navigate to the module's main configuration form.
  • Click the "Custom paths" vertical tab and enter values for "Custom jQuery path" and "Custom jQuery version" and click "Save configuration".
  • Result: on the reloaded page, the custom version you entered appears as the default value for "Default jQuery version" even though you didn't select it there (indeed, on the previous screen, it wasn't an option).
  • Select a different value for "Default jQuery version" and click "Save configuration".
  • Result: on the reloaded page, the custom version still appears as the default value for "Default jQuery version". (While this might prompt a user to conclude their new selection wasn't saved, in fact is was.)

Proposed resolution

A possible fix would be to remove the code involved:

  $custom_jquery = variable_get('jquery_update_custom_version_jquery', FALSE);
  if (!empty($custom_jquery)) {
    $default_version = $custom_jquery;
  }

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

4.0

Component

Code

Created by

🇨🇦Canada nedjo

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @nedjo
  • 🇸🇰Slovakia poker10

    Result: on the reloaded page, the custom version you entered appears as the default value for "Default jQuery version" even though you didn't select it there

    Actually this seems to be caused by this code (not the one you mentioned):

    function jquery_update_settings_form_submit_late($form, &$form_state) {
      // Special handling for the first time a custom jQuery version is configured.
      if (isset($form_state['values']['jquery_update_custom_version_jquery'])) {
        $custom_version = $form_state['values']['jquery_update_custom_version_jquery'];
        // If the custom version is not yet available as an option in the form, set
        // it as the default anyway. This avoids new custom versions requiring two
        // submissions of the admin form.
        if (!in_array($custom_version, array_keys($form['jquery_update_jquery_version']['#options']))) {
          variable_set('jquery_update_jquery_version', $custom_version);
        }
      }
    }
    

    Everytime the config form is saved and there is a NEW jquery_update_custom_version_jquery version set (which is not present in the options array yet), then the jquery_update_jquery_version variable is set to this newly added custom jquery version. It seems to me as an intended action, regarding that comment.

    However, while testing this, I have run into one interesting use-case:

    1. clean install - default jquery version = 1.4 (core) and default theme (bartik) jquery version = Site Default
    2. set custom jquery path and version (for example 3.6.0)
    3. bartik is now using 3.6.0
    4. return to the config form and remove custom jquery path and version, save form
    5. the default jquery version is automatically reverted to the jquery 2.2 and the default theme (bartin) jquery version is reverted automatically to the 1.12 (so none of the settings is back on 1.4, which should be ok, but it is weird they are reset differently, which can be confusing)

    Not sure if we can examine and handle it here, or create a new issue.

  • 🇸🇰Slovakia poker10

    Debugged this a bit more and the "problem" mentioned by me is caused by the fact the theme_get_setting('jquery_update_jquery_version') returns an empty string if the theme has "Site Default" settings, see:

    function jquery_update_get_version_options($empty = TRUE) {
      $options = array_merge(array(
        '' => t('Site default (!version)', array(
          '!version' => variable_get('jquery_update_jquery_version', JQUERY_UPDATE_DEFAULT_JQUERY_VERSION),
        )),
        'default' => t('1.4 (Drupal core)'),
      ), jquery_update_get_supported_version_options());
      if (!$empty) {
        unset($options['']);
      }
      return $options;
    }
    

    So if you had this setting:
    - default jquery version = 1.4 (core) and default theme (bartik) jquery version = Site Default

    Then changed to this:
    - default jquery version = 3.6.0 (custom) and default theme (bartik) jquery version = Site Default

    Then removed the custom path and version and visit bartik theme, then the variable jquery_update_jquery_version still has 3.6.0 set. However this is no longer considered valid, because it is not custom version anymore (custom path and version are deleted) and it is not in the array returned by jquery_update_get_versions(). Therefore it is converted by _jquery_update_convert_settings_to_supported_versions() and _jquery_update_map_to_supported_version() to the supported version as follows:

    function _jquery_update_map_to_supported_version($version) {
      if ($version == 'default') {
        return $version;
      }
      if (version_compare($version, '2', '<')) {
        return '1.12';
      }
      if (version_compare($version, '1', '>')) {
        return '2.2';
      }
      return FALSE;
    }
    

    This will change 3.6.0 (default jquery version of the site) to 2.2 and empty string (bartiks' setting of "Site Default") to 1.12. Thus the difference. Not sure if we want to do something with this.

  • 🇮🇱Israel heyyo Jerusalem

    We have the same issue after upgrading to jquery_update 7.x-4.1.
    We set local custom paths for JQuery (3.5.1) and JQuery Migrate.
    For some reason, the front theme is reverted to JQuery 1.12 instead of 3.5.1 after a while.

  • 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺

    #3317322: jQuery version for theme not loading as selected if custom path is set is where we added the slightly unusual "late submit handler" for the admin settings form.

    https://git.drupalcode.org/project/jquery_update/-/blob/7.x-4.1/jquery_u...

    /**
     * Late submit handler for settings form.
     */
    function jquery_update_settings_form_submit_late($form, &$form_state) {
      // Special handling for the first time a custom jQuery version is configured.
      if (isset($form_state['values']['jquery_update_custom_version_jquery'])) {
        $custom_version = $form_state['values']['jquery_update_custom_version_jquery'];
        // If the custom version is not yet available as an option in the form, set
        // it as the default anyway. This avoids new custom versions requiring two
        // submissions of the admin form.
        if (!in_array($custom_version, array_keys($form['jquery_update_jquery_version']['#options']))) {
          variable_set('jquery_update_jquery_version', $custom_version);
        }
      }
    }
    

    That caters for a specific scenario where it's desirable for the newly submitted custom jQuery version to become the default (without having to submit the settings form multiple times).

    The idea is that the submit handler only kicks in if the newly submitted custom version is not yet in the list of options from which the default can be selected.

    Is there a bug in that functionality? Unless I'm missing something it shouldn't make unexpected changes to the default unless a new custom version has been submitted.

    It makes sense that there may be situations where setting a newly submitted custom version as the default is not the desired outcome.

    How would we cater for that? Perhaps checkbox to accompany the custom version that says "this should be the site default" or something like that?

    I am not a UX expert so open to suggestions on that front, and on the implementation.

    The motivation for adding the handler was that tests failed when a new custom version was configured, but it required a second submission of the settings form to select this as the default.

    @heyyo does this sound like the same problem you've seen, or might there be something else going on? i.e. are things changing when the custom version is updated, or without that being changed?

  • 🇧🇾Belarus q2_faith

    This will change 3.6.0 (default jquery version of the site) to 2.2 and empty string (bartiks' setting of "Site Default") to 1.12.

    We have such behaviour. A website has "jquery_update_jquery_version" and "jquery_update_custom_version_jquery" that were set by custom paths - 3.5.1. But then, it reverted to 1.12.

  • 🇮🇱Israel heyyo Jerusalem

    After some more digging, we finally understood what's happening, in our website we are using domain_access module.
    With sub modules domain_conf, and domain_theme. So it's in some domains we still had values for variable jquery_update_jquery_version with value 1.8. So the simple fact to browse one of those domains, was enough to revert the jquery_update_jquery_version to default 1.12.

    This is the solution we found executing this drush command:
    drush dc-set all jquery_update_jquery_version 3.5.1

    Which delete all domain specific configuration.

  • Status changed to Closed: won't fix over 1 year ago
  • 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺

    Okay, thanks for the reports and the details of workarounds for domain-related issues.

    I don't think there's anything "actionable" in terms of fixing code in the module here, so I'm going to close this.

    Please re-open if you have steps to reproduce a specific issue that you think the module could fix.

  • Status changed to Active over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 7.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    10 pass
  • @volha_si opened merge request.
  • Hi,
    after some investigation I found that here we have two separate problems (not domain-connected):

    1) The default version is being set equal to custom on the custom version submission if the custom version is new (or 1.4, but let's discuss it here 🐛 When custom version is set to 1.4, the default changes and 1.4 is dublicated in the list of default version Needs work ).
    As mcdruid explained, it works as designed but it's still confusing.
    So I propose just to add a note that the default theme will be set equal to custom after submitting.

    2) 'Default jQuery version' selector always shows the value of the Custom one (jquery_update_custom_version_jquery) instead of the Default (jquery_update_jquery_version) even if they are not equal.

    STR:

    1. 1. Set the custom version to 3.5 and save configuration.
    2. 2. Change the default version from 3.5 to 2.2 and save configuration.

    AR: the default version is still shown as 3.5. But in DB jquery_update_jquery_version is set to 2.2.

    This issue should be fixed, as proposed in the issue description, because it makes no sense to show here the custom version under the title 'Default jQuery version'.
    I've made the fix, review needed.

  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work 10 months ago
  • 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺

    Thanks for the patch/MR.

    Can we add a test which demonstrates what problem we're fixing here please?

  • 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺
Production build 0.71.5 2024