Fixes #1490: Busy-loop when fetching registration form fails

CHANGES.md

@@ -2,8 +2,10 @@
 
 ## 10.1.2 (Unreleased)
 
+- #1490: Busy-loop when fetching registration form fails
 - #1556: Can't switch to registration form afrer logout
 - #3137: Various UI/UX bugfixes regarding the registration form
+- XEP-0437: Room Activity Indicators (RAI) optimizations
 
 ## 10.1.1 (2023-02-15)
 

src/plugins/register/panel.js

@@ -3,12 +3,10 @@ import tplFormInput from "templates/form_input.js";
 import tplFormUrl from "templates/form_url.js";
 import tplFormUsername from "templates/form_username.js";
 import tplRegisterPanel from "./templates/register_panel.js";
-import tplSpinner from "templates/spinner.js";
 import { CustomElement } from 'shared/components/element.js';
 import { __ } from 'i18n';
 import { _converse, api, converse } from "@converse/headless/core.js";
 import { initConnection } from '@converse/headless/utils/init.js';
-import { render } from 'lit';
 import { setActiveForm } from './utils.js';
 import { webForm2xForm } from "@converse/headless/utils/form";
 
@@ -22,6 +20,7 @@ const u = converse.env.utils;
 const CHOOSE_PROVIDER = 0;
 const FETCHING_FORM = 1;
 const REGISTRATION_FORM = 2;
+const REGISTRATION_FORM_ERROR = 3;
 
 
 /**
@@ -34,10 +33,18 @@ class RegisterPanel extends CustomElement {
     static get properties () {
         return {
             status : { type: String },
-            error_message: { type: String },
+            alert_message: { type: String },
+            alert_type: { type: String },
         }
     }
 
+    constructor () {
+        super();
+        this.alert_type = 'info';
+        this.setErrorMessage = (m) => this.setMessage(m, 'danger');
+        this.setFeedbackMessage = (m) => this.setMessage(m, 'info');
+    }
+
     initialize () {
         this.reset();
         this.listenTo(_converse, 'connectionInitialized', () => this.registerHooks());
@@ -54,6 +61,11 @@ class RegisterPanel extends CustomElement {
         return tplRegisterPanel(this);
     }
 
+    setMessage(message, type) {
+        this.alert_type = type;
+        this.alert_message = message;
+    }
+
     /**
      * Hook into Strophe's _connect_cb, so that we can send an IQ
      * requesting the registration fields.
@@ -64,17 +76,14 @@ class RegisterPanel extends CustomElement {
         conn._connect_cb = (req, callback, raw) => {
             if (!this._registering) {
                 connect_cb(req, callback, raw);
-            } else {
+            } else if (this.getRegistrationFields(req, callback)) {
-                if (this.getRegistrationFields(req, callback)) {
+                this._registering = false;
-                    this._registering = false;
-                }
             }
         };
     }
 
     /**
      * Send an IQ stanza to the XMPP server asking for the registration fields.
-     * @private
      * @method _converse.RegisterPanel#getRegistrationFields
      * @param { Strophe.Request } req - The current request
      * @param { Function } callback - The callback function
@@ -87,7 +96,7 @@ class RegisterPanel extends CustomElement {
         if (!body) { return; }
         if (conn._proto._connect_cb(body) === Strophe.Status.CONNFAIL) {
             this.status = CHOOSE_PROVIDER;
-            this.error_message = __("Sorry, we're unable to connect to your chosen provider.");
+            this.setErrorMessage(__("Sorry, we're unable to connect to your chosen provider."));
             return false;
         }
         const register = body.getElementsByTagName("register");
@@ -98,10 +107,11 @@ class RegisterPanel extends CustomElement {
         }
         if (register.length === 0) {
             conn._changeConnectStatus(Strophe.Status.REGIFAIL);
-            this.error_message =
+            this.alert_type = 'danger';
+            this.setErrorMessage(
                 __("Sorry, the given provider does not support in "+
                    "band account registration. Please try with a "+
-                   "different provider.");
+                   "different provider."));
             return true;
         }
         // Send an IQ stanza to get all required data fields
@@ -115,24 +125,17 @@ class RegisterPanel extends CustomElement {
 
     /**
      * Handler for {@link _converse.RegisterPanel#getRegistrationFields}
-     * @private
      * @method _converse.RegisterPanel#onRegistrationFields
      * @param { XMLElement } stanza - The query stanza.
      */
     onRegistrationFields (stanza) {
         if (stanza.getAttribute("type") === "error") {
-            _converse.connection._changeConnectStatus(
+            this.reportErrors(stanza);
-                Strophe.Status.REGIFAIL,
+            if (api.settings.get('registration_domain')) {
-                __('Something went wrong while establishing a connection with "%1$s". '+
+                this.status = REGISTRATION_FORM_ERROR;
-                   'Are you sure it exists?', this.domain)
+            } else {
-            );
+                this.status = CHOOSE_PROVIDER;
-            return false;
+            }
-        }
-        if (stanza.getElementsByTagName("query").length !== 1) {
-            _converse.connection._changeConnectStatus(
-                Strophe.Status.REGIFAIL,
-                "unknown"
-            );
             return false;
         }
         this.setFields(stanza);
@@ -160,7 +163,6 @@ class RegisterPanel extends CustomElement {
     /**
      * Event handler when the #converse-register form is submitted.
      * Depending on the available input fields, we delegate to other methods.
-     * @private
      * @param { Event } ev
      */
     onFormSubmission (ev) {
@@ -175,7 +177,6 @@ class RegisterPanel extends CustomElement {
 
     /**
      * Callback method that gets called when the user has chosen an XMPP provider
-     * @private
      * @method _converse.RegisterPanel#onProviderChosen
      * @param { HTMLElement } form - The form that was submitted
      */
@@ -186,7 +187,6 @@ class RegisterPanel extends CustomElement {
 
     /**
      * Fetch a registration form from the requested domain
-     * @private
      * @method _converse.RegisterPanel#fetchRegistrationForm
      * @param { String } domain_name - XMPP server domain
      */
@@ -203,30 +203,9 @@ class RegisterPanel extends CustomElement {
         return false;
     }
 
-    giveFeedback (message, klass) {
-        let feedback = this.querySelector('.reg-feedback');
-        if (feedback !== null) {
-            feedback.parentNode.removeChild(feedback);
-        }
-        const form = this.querySelector('form');
-        form.insertAdjacentHTML('afterbegin', '<span class="reg-feedback"></span>');
-        feedback = form.querySelector('.reg-feedback');
-        feedback.textContent = message;
-        if (klass) {
-            feedback.classList.add(klass);
-        }
-    }
-
-    showSpinner () {
-        const form = this.querySelector('form');
-        render(tplSpinner(), form);
-        return this;
-    }
-
     /**
      * Callback function called by Strophe whenever the connection status changes.
      * Passed to Strophe specifically during a registration attempt.
-     * @private
      * @method _converse.RegisterPanel#onConnectStatusChanged
      * @param { integer } status_code - The Strophe.Status status code
      */
@@ -246,11 +225,11 @@ class RegisterPanel extends CustomElement {
         } else if (status_code === Strophe.Status.REGISTERED) {
             log.debug("Registered successfully.");
             _converse.connection.reset();
-            this.showSpinner();
 
             if (["converse/login", "converse/register"].includes(_converse.router.history.getFragment())) {
                 _converse.router.navigate('', {'replace': true});
             }
+            setActiveForm('login');
 
             if (this.fields.password && this.fields.username) {
                 // automatically log the user in
@@ -259,11 +238,9 @@ class RegisterPanel extends CustomElement {
                     this.fields.password,
                     _converse.onConnectStatusChanged
                 );
-                setActiveForm('login');
+                this.setFeedbackMessage(__('Now logging you in'));
-                _converse.router.navigate('');
-                this.giveFeedback(__('Now logging you in'), 'info');
             } else {
-                _converse.giveFeedback(__('Registered successfully'));
+                this.setFeedbackMessage(__('Registered successfully'));
             }
             this.reset();
         }
@@ -308,7 +285,6 @@ class RegisterPanel extends CustomElement {
     /**
      * Renders the registration form based on the XForm fields
      * received from the XMPP server.
-     * @private
      * @method _converse.RegisterPanel#renderRegistrationForm
      * @param { XMLElement } stanza - The IQ stanza received from the XMPP server.
      */
@@ -320,17 +296,16 @@ class RegisterPanel extends CustomElement {
     /**
      * Report back to the user any error messages received from the
      * XMPP server after attempted registration.
-     * @private
      * @method _converse.RegisterPanel#reportErrors
      * @param { XMLElement } stanza - The IQ stanza received from the XMPP server
      */
     reportErrors (stanza) {
         const errors = Array.from(stanza.querySelectorAll('error'));
         if (errors.length) {
-            this.error_message = errors.reduce((result, e) => `${result}\n${e.textContent}`, '');
+            this.setErrorMessage(errors.reduce((result, e) => `${result}\n${e.textContent}`, ''));
         } else {
-            this.error_message = __('The provider rejected your registration attempt. '+
+            this.setErrorMessage(__('The provider rejected your registration attempt. '+
-                'Please check the values you entered for correctness.');
+                'Please check the values you entered for correctness.'));
         }
     }
 
@@ -356,7 +331,6 @@ class RegisterPanel extends CustomElement {
     /**
      * Handler, when the user submits the registration form.
      * Provides form error feedback or starts the registration process.
-     * @private
      * @method _converse.RegisterPanel#submitRegistrationForm
      * @param { HTMLElement } form - The HTML form that was submitted
      */
@@ -378,8 +352,8 @@ class RegisterPanel extends CustomElement {
         this.setFields(iq.tree());
     }
 
-    /* Stores the values that will be sent to the XMPP server during attempted registration.
+    /**
-     * @private
+     * Stores the values that will be sent to the XMPP server during attempted registration.
      * @method _converse.RegisterPanel#setFields
      * @param { XMLElement } stanza - the IQ stanza that will be sent to the XMPP server.
      */
@@ -428,7 +402,6 @@ class RegisterPanel extends CustomElement {
      * Callback method that gets called when a return IQ stanza
      * is received from the XMPP server, after attempting to
      * register a new user.
-     * @private
      * @method _converse.RegisterPanel#reportErrors
      * @param { XMLElement } stanza - The IQ stanza.
      */

src/plugins/register/templates/register_panel.js

@@ -1,17 +1,16 @@
 import tplRegistrationForm from './registration_form.js';
 import tplSpinner from 'templates/spinner.js';
+import tplSwitchForm from './switch_form.js';
 import { __ } from 'i18n';
 import { api } from '@converse/headless/core';
 import { html } from 'lit';
 
 const tplFormRequest = (el) => {
     const default_domain = api.settings.get('registration_domain');
-    const i18n_fetch_form = __("Hold tight, we're fetching the registration form…");
     const i18n_cancel = __('Cancel');
     return html`
         <form id="converse-register" class="converse-form no-scrolling" @submit=${ev => el.onFormSubmission(ev)}>
             ${tplSpinner({ 'classes': 'hor_centered' })}
-            <p class="info">${i18n_fetch_form}</p>
             ${default_domain
                 ? ''
                 : html`
@@ -73,13 +72,15 @@ const tplChooseProvider = (el) => {
 const CHOOSE_PROVIDER = 0;
 const FETCHING_FORM = 1;
 const REGISTRATION_FORM = 2;
+const REGISTRATION_FORM_ERROR = 3;
 
 export default (el) => {
     return html`
         <converse-brand-logo></converse-brand-logo>
-        ${ el.error_message ? html`<div class="alert alert-danger" role="alert">${el.error_message}</div>` : '' }
+        ${ el.alert_message ? html`<div class="alert alert-${el.alert_type}" role="alert">${el.alert_message}</div>` : '' }
         ${el.status === CHOOSE_PROVIDER ? tplChooseProvider(el) : ''}
         ${el.status === FETCHING_FORM ? tplFormRequest(el) : ''}
         ${el.status === REGISTRATION_FORM ? tplRegistrationForm(el) : ''}
+        ${el.status === REGISTRATION_FORM_ERROR ? tplSwitchForm() : '' }
     `;
 };

src/plugins/register/templates/registration_form.js

@@ -1,12 +1,11 @@
+import tplSwitchForm from './switch_form.js';
 import { __ } from 'i18n';
 import { api } from '@converse/headless/core';
 import { html } from 'lit';
 
 export default (el) => {
     const i18n_choose_provider = __('Choose a different provider');
-    const i18n_has_account = __('Already have a chat account?');
     const i18n_legend = __('Account Registration:');
-    const i18n_login = __('Log in here');
     const i18n_register = __('Register');
     const registration_domain = api.settings.get('registration_domain');
 
@@ -34,10 +33,7 @@ export default (el) => {
                               @click=${ev => el.renderProviderChoiceForm(ev)}
                           />
                       `}
-                <div class="switch-form">
+                ${ tplSwitchForm() }
-                    <p>${i18n_has_account}</p>
-                    <p><a class="login-here toggle-register-login" href="#converse/login">${i18n_login}</a></p>
-                </div>
             </fieldset>
         </form>
     `;

src/plugins/register/templates/switch_form.js

@@ -0,0 +1,12 @@
+import { __ } from 'i18n';
+import { html } from 'lit';
+
+export default () => {
+    const i18n_has_account = __('Already have a chat account?');
+    const i18n_login = __('Log in here');
+    return html`
+        <div class="switch-form">
+            <p>${i18n_has_account}</p>
+            <p><a class="login-here toggle-register-login" href="#converse/login">${i18n_login}</a></p>
+        </div>`;
+}