The block config's "cancel" button breaks after a Pieform validation failure

Bug #1560329 reported by Aaron Wells on 2016-03-22
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Medium
Aaron Wells
15.10
Medium
Aaron Wells
16.04
Medium
Aaron Wells
16.10
Medium
Aaron Wells

Bug Description

To replicate:

1. Drag a new "external media" block into your page.
2. In the block config modal that opens up, hit "submit" without filling in the required "URL or embed code" field
3. Press "Submit"
4. Wait for the form to reload and display the warning message telling you about the required field
5. Once that loads up, press the "Cancel" or "Remove" button/link next to the "Submit" button.

Expected result: The block config modal window closes, and the new external media block is no longer on your page

Actual result: The screen reloads, and you now have an empty "external media" block on your page

The reason this is happening, is because the Pieforms "cancel" button by default just reloads the form's action URL, or the cancel button's "goto" URL. When we load up the block config modal, we use Javascript code to rewire the cancel button so that it instead closes the modal and, if the block is new, deletes the block.

However, when Pieforms fails validation, Pieforms actually removes the current form displayed on the page, and replaces it with a new form that contains the validation errors. Because this is a completely different form object, the "rewiring" that we did when we opened the modal no longer applies.

Aaron Wells (u-aaronw) wrote :

Fortunately, this bug is not very noticeable because few of the blocks use required fields, and the failure mode only happens if you hit the "cancel" button, and even then, if you're not paying attention you may not notice the difference between the modal closing and the page reloading.

The most noticeable side effect is that it can lead to the creation of "empty" blocks on your page, like an External Feed block with no External Feed URL, or an External Video block with no video.

Reviewed: https://reviews.mahara.org/6226
Committed: https://git.mahara.org/mahara/mahara/commit/ac21cd81495d12f277a55d97245ed2dc0bc414fd
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit ac21cd81495d12f277a55d97245ed2dc0bc414fd
Author: Aaron Wells <email address hidden>
Date: Tue Mar 22 18:56:14 2016 +1300

Better handling of Pieforms validation failure in block modal

Bug 1560329: We were neglecting to rewire the "cancel/remove"
button on a block config after a Pieforms validation failure.
A pieforms validation failure (when you're using Pieforms'
jsforms feature) actually replaces the current form with a
new one, rather than just inserting error messages into the
existing form.

Change-Id: Ie3fa440595b08cc5a49cb89cb7ca95bcfa02c2db
behatnotneeded: Not easy to test with Behat

Reviewed: https://reviews.mahara.org/7076
Committed: https://git.mahara.org/mahara/mahara/commit/ce695f4b453fa8551cbff532cf578e550e42f4f0
Submitter: Robert Lyon (<email address hidden>)
Branch: 16.10_STABLE

commit ce695f4b453fa8551cbff532cf578e550e42f4f0
Author: Aaron Wells <email address hidden>
Date: Tue Mar 22 18:56:14 2016 +1300

Better handling of Pieforms validation failure in block modal

Bug 1560329: We were neglecting to rewire the "cancel/remove"
button on a block config after a Pieforms validation failure.
A pieforms validation failure (when you're using Pieforms'
jsforms feature) actually replaces the current form with a
new one, rather than just inserting error messages into the
existing form.

Change-Id: Ie3fa440595b08cc5a49cb89cb7ca95bcfa02c2db
behatnotneeded: Not easy to test with Behat
(cherry picked from commit ac21cd81495d12f277a55d97245ed2dc0bc414fd)

Mahara Bot (dev-mahara) wrote :

Patch for "15.10_STABLE" branch: https://reviews.mahara.org/7078

Reviewed: https://reviews.mahara.org/7078
Committed: https://git.mahara.org/mahara/mahara/commit/3813ea8e65d1a1aceec7cb4bd722f1f7df027f3c
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit 3813ea8e65d1a1aceec7cb4bd722f1f7df027f3c
Author: Aaron Wells <email address hidden>
Date: Tue Mar 22 18:56:14 2016 +1300

Better handling of Pieforms validation failure in block modal

Bug 1560329: We were neglecting to rewire the "cancel/remove"
button on a block config after a Pieforms validation failure.
A pieforms validation failure (when you're using Pieforms'
jsforms feature) actually replaces the current form with a
new one, rather than just inserting error messages into the
existing form.

Change-Id: Ie3fa440595b08cc5a49cb89cb7ca95bcfa02c2db
behatnotneeded: Not easy to test with Behat
(cherry picked from commit ac21cd81495d12f277a55d97245ed2dc0bc414fd)
(cherry picked from commit ce695f4b453fa8551cbff532cf578e550e42f4f0)

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/7077
Committed: https://git.mahara.org/mahara/mahara/commit/f74fa0710baaeae199738fd688622b5624cfe7a6
Submitter: Robert Lyon (<email address hidden>)
Branch: 16.04_STABLE

commit f74fa0710baaeae199738fd688622b5624cfe7a6
Author: Aaron Wells <email address hidden>
Date: Tue Mar 22 18:56:14 2016 +1300

Better handling of Pieforms validation failure in block modal

Bug 1560329: We were neglecting to rewire the "cancel/remove"
button on a block config after a Pieforms validation failure.
A pieforms validation failure (when you're using Pieforms'
jsforms feature) actually replaces the current form with a
new one, rather than just inserting error messages into the
existing form.

Change-Id: Ie3fa440595b08cc5a49cb89cb7ca95bcfa02c2db
behatnotneeded: Not easy to test with Behat
(cherry picked from commit ac21cd81495d12f277a55d97245ed2dc0bc414fd)
(cherry picked from commit ce695f4b453fa8551cbff532cf578e550e42f4f0)

Robert Lyon (robertl-9) on 2016-10-21
Changed in mahara:
milestone: 16.04.1 → none
Robert Lyon (robertl-9) on 2016-10-25
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers