Uploaded image for project: 'XWiki Platform'
  1. XWiki Platform
  2. XWIKI-24621

Notification watch settings can silently fail to be saved

    XMLWordPrintable

Details

    • Bug
    • Resolution: Unresolved
    • Minor
    • None
    • 16.5.0, 18.3.0
    • Notifications
    • None
    • Unknown

    Description

      Steps to reproduce

      1. Log in as a user allowed to change their notification settings.
      2. Go to any page and click the watch (bell) button in the page menu.
      3. In the modal that opens, select an option (for instance "Watch this page", or "Unwatch this page and watch the space instead") and confirm.
      4. Do the same while the request cannot succeed, for example by denying the REST request (offline browser, read-only wiki, or a user without the rights).

      Expected result

      Either the setting is applied and the page reloads, or, when the request fails, the modal gives up cleanly: the selection is cleared, the button is disabled again and the failure is reported.

      Actual result

      Every confirmation throws in the browser console:

      Uncaught TypeError: promise.done(...).error is not a function
      

      The success path still works by accident, because the REST request has already been sent and the done() callback has already been registered when the exception is thrown. So the problem stays invisible as long as nothing fails.

      But the failure callback is never registered, so when the request does fail nothing at all happens: the modal stays open with the option still selected, the page does not reload, and no error is reported anywhere – not even in the console. The user is left believing their watch or block choice was saved when it was not, and the only way to find out is to open the notification filter preferences in their profile.

      The cause is in XWiki.Notifications.Code.NotificationsWatchUIX (JSX object "Handle watch settings change"), which calls error() on the jqXHR returned by $.ajax():

      promise.done(function () {
        removeAnySelection();
        window.location.reload();
      }).error(function (data) {
        removeAnySelection();
        console.error("Error when processing the request: " + data);
      });
      

      jqXHR.success(), jqXHR.error() and jqXHR.complete() were removed in jQuery 3.0. jQuery Migrate 3.x re-added them, which is why this used to work. jQuery Migrate 4.x no longer does, so this broke in 18.3.0, when the platform moved to jQuery 4.0.0 and jQuery Migrate 4.0.2 (XWIKI-23979).

      Second problem in the same handler: the combined options fire both requests at once

      For the two combined options – "Unwatch this page and watch the space instead" and its block equivalent – the second REST call is fired immediately instead of after the first one has completed:

      var firstPromise = performChange('unwatchPage');
      if (firstPromise !== 'undefined') {
        promise = firstPromise.pipe(performChange('watchSpace'))
      }
      

      performChange('watchSpace') is called on that line, so its request leaves at the same time as the unwatchPage one, and pipe() receives a jqXHR object instead of a callback function (jQuery ignores non-function arguments). Consequences, from a user point of view:

      • the returned promise resolves as soon as the first request completes, so window.location.reload() can fire while the space-level request is still in flight. The browser then cancels it and the space-level watch or block is silently not applied – the user asked to watch the space and ends up watching nothing;
      • the two requests are unordered even though they modify the same user notification filter preferences;
      • this is a plausible source of flickering for NotificationsSettingsIT, which asserts the resulting filter right after using that option.

      Deferred.pipe() is also deprecated, and is one of the remaining JQMIGRATE console messages tracked by XWIKI-23202.

      Finally, both guards in that function compare against the string 'undefined' instead of testing for undefined, so they never trigger. When performChange() returns nothing (unsupported option) the code throws Cannot read properties of undefined (reading 'done') instead of doing nothing.

      Proposed fix

      In xwiki-platform-notifications-ui, file src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml, JSX object "Handle watch settings change": chain the second request with then(), replace the removed error() by fail(), and use truthy guards.

      var handleSubmit = function () {
        var chosenOption = $('#watchModal input[type="radio"]:checked').val();
        var promise;
        if (chosenOption == 'unwatchPageWatchSpace') {
          promise = performChange('unwatchPage');
          if (promise) {
            promise = promise.then(function () {
              return performChange('watchSpace');
            });
          }
        } else if (chosenOption == 'unblockPageBlockSpace') {
          promise = performChange('unblockPage');
          if (promise) {
            promise = promise.then(function () {
              return performChange('blockSpace');
            });
          }
        } else {
          promise = performChange(chosenOption);
        }
        if (promise) {
          promise.done(function () {
            removeAnySelection();
            window.location.reload();
          }).fail(function (data) {
            removeAnySelection();
            console.error("Error when processing the request: " + data);
          });
        }
      };
      

      The fix must not rely on jQuery Migrate being present: it is bundled only as a temporary compatibility layer for extensions that have not been migrated to jQuery 4 yet.

      Attachments

        Activity

          People

            vmassol Vincent Massol
            vmassol Vincent Massol
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated: