enketo-core icon indicating copy to clipboard operation
enketo-core copied to clipboard

Uninformative error when initialising a form with no active pages

Open garethbowen opened this issue 4 years ago • 1 comments

If you try and render a form with no active pages (eg: one input set to relevant="false()") the error message you get doesn't help diagnose the issue.

For example, this form:

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <h:head>
    <h:title>Test Issue 4279 A: timer with relevant</h:title>
    <model>
      <instance>
        <issue_4279_a delimiter="#" id="issue_4279_a" prefix="J1!issue_4279_a!" version="2018-03-08 17-42">
          <group>
            <timer/>
          </group>
          <meta>
            <instanceID/>
          </meta>
        </issue_4279_a>
      </instance>
      <instance id="contact-summary"/>
      <bind nodeset="/issue_4279_a/group/timer" readonly="true()" relevant="false()" type="string"/>
      <bind calculate="concat('uuid:', uuid())" nodeset="/issue_4279_a/meta/instanceID" readonly="true()" type="string"/>
    </model>
  </h:head>
  <h:body class="pages">
    <group appearance="field-list" ref="/issue_4279_a/group">
      <label>Testing Timer</label>
      <input appearance="countdown-timer" ref="/issue_4279_a/group/timer">
        <label>Text for timer</label>
      </input>
    </group>
  </h:body>
</h:html> 

Causes this stack trace to be logged and returned in load errors:

TypeError: Cannot read property 'scrollIntoView' of undefined
    at Object.focusOnFirstQuestion
    at Object.flipTo
    at Object.flipToFirst
    at Object.init
    at Form.init
    ...

What's happening is the flipToFirst function is passing the 0th element of an empty array (ie: undefined) to the flipTo function which calls the focusOnFirstQuestion. Everything is null safe until the very last line which throws the Error. The Error is caught, logged, and returned in the Form.init.

I can see two ways to make this more useful:

  1. If this form is technically valid but useless, then add a null check to the focusOnFirstQuestion function and allow the empty form to be rendered. The user will then be able to work out what's wrong and fix the issue.
  2. If this form is invalid then when we discover that $activePages is empty create a new Error with a descriptive error message and append this to loadErrors and return without logging. This allows the calling application to handle it gracefully and improves the usefulness of the message shown to the user.

I'm happy to provide a PR to fix this but would appreciate any feedback on what the correct approach is.

garethbowen avatar Sep 10 '19 00:09 garethbowen

I think this issue has been "fixed" (though perhaps unintentionally). The important thing that has changed between when this issue was logged (4.41.6) and now is how the page.activePages array is filtered.

Previously the old _updateAllActive function would keep a page if this evaluated to true:

$this.find( '.question:not(.disabled)' ).length > 0

The updated _updateAllActive function replaced that check with the following:

el.querySelector( '.question:not(.disabled)' )

When I load the sample form provided above using the latest version of enketo-core, I do not see the error. Debugging into the execution I can see that the page is added to activePages because el.querySelector( '.question:not(.disabled)' ) returns an empty object ({}) that is evaluated as true. (Meaning that pages without enabled questions could still be considered "active".)

Additionally, (and probably more relevant for this issue), the updated _flipTo function only ever calls _focusOnFirstQuestion if either current or pageEl exist (which would be false anyway even if activePages was empty).

jkuester avatar Aug 18 '21 22:08 jkuester