Error shown when changing your profile icon to default "Standard or external avatar"

Bug #1311940 reported by Aaron Wells
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Unassigned
1.10
Fix Released
High
Unassigned
1.9
Fix Released
High
Unassigned

Bug Description

To replicate:

1. Go to Content->Profile pictures
2. Upload a profile picture
3. Select that profile picture as your "Default" and click "Set default"
4. Now, select the anonymous profile icon ("Standard or external avatar") and click "Set default"

You'll see a red warning box at the top of the form that says "There was an error with submitting this form. Please check the marked fields and try again."

Tags: regression
Revision history for this message
Aaron Wells (u-aaronw) wrote :
Revision history for this message
Aaron Wells (u-aaronw) wrote :

Regression caused by this commit:

commit 80375b1412f6ba021c85641edb849c1014fc0aa3
Author: Jono Mingard <email address hidden>
Date: Tue Jan 28 15:00:49 2014 +1300

    Added top-level form errors to complement specific ones (Bug #1262483)

    Pieforms now adds a global error message if there are any validation errors
    in a form, in addition to the messages beside each element. This is
    modified from the existing jserrormessage

    Change-Id: I15b9f4238ec3e5b1e6cb7fcff0514855565f0364
    Signed-off-by: Jono Mingard <email address hidden>

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Okay, the problem here is that Jono's code change doesn't support an obscure Pieforms feature that lets a form have multiple submit buttons, each of which gets its own submit function.

If you create a form with multiple submit buttons, and you create a function called {$formname}_submit_{$buttonname}(), then that function will be called by pieforms if the user clicks that button. This is used on exactly two forms: The profile picture picker, and the admin/users/suspended.php form for unsuspended, unexpiring, or deleting a user.

Notably, neither of these forms has a {$formname}_submit() method. And the way Pieforms is written, it's supposed to return after hitting that _submit() function, and if it doesn't return, it gets to the point where it displays this error message.

So probably the solution here, is that if it goes to a button submit function, it Pieforms should return.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/3285

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/3286

Revision history for this message
Aaron Wells (u-aaronw) wrote :

An additional wrinkle in the problem is that those button-level submit callback functions are supposed to redirect() instead of returning, and the profile picture one was not redirecting, when you chose the default profile pic.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/3285
Committed: http://gitorious.org/mahara/mahara/commit/8c19958a783d3acc7620641767bda0a160237595
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 8c19958a783d3acc7620641767bda0a160237595
Author: Aaron Wells <email address hidden>
Date: Thu Apr 24 14:17:27 2014 +1200

Prevent global form error from incorrectly showing up on forms with button-submit functions

Bug 1311940: There's an obscure pieforms feature where a form can have multiple submit
buttons, and each submit button can have its own submit callback function named
{formname}_submit_{buttonname}. These functions are supposed to redirect at the end.
If they don't, due to a bug in the implementation of the global form message we currently show
an error message at the top of the form saying there was something wrong with the user's
submission. Instead, we should just let the form go through, and log a debug message for
the devs.

Change-Id: Ib0e9b7e2f58737c9c736bdd1c033bc020985a60b

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/3286
Committed: http://gitorious.org/mahara/mahara/commit/6e68097cb2f6da18eccba00fdaf5dcd6038cf6f2
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 6e68097cb2f6da18eccba00fdaf5dcd6038cf6f2
Author: Aaron Wells <email address hidden>
Date: Thu Apr 24 14:28:24 2014 +1200

No need to show a different message when setting profile icon to the default

Bug 1311940: Also, a pieform button-level submit function is meant to redirect
at the end every time. And this one was not.

Change-Id: I7b71311df9aa29db9b5d7c892a0ee8b13442f7b3

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "1.9_STABLE" branch: https://reviews.mahara.org/3299

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Patch for "1.9_STABLE" branch: https://reviews.mahara.org/3298

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/3298
Committed: http://gitorious.org/mahara/mahara/commit/290e743503c6d6f4378426d093f7fd7f075b1584
Submitter: Robert Lyon (<email address hidden>)
Branch: 1.9_STABLE

commit 290e743503c6d6f4378426d093f7fd7f075b1584
Author: Aaron Wells <email address hidden>
Date: Thu Apr 24 14:17:27 2014 +1200

Prevent global form error from incorrectly showing up on forms with button-submit functions

Bug 1311940: There's an obscure pieforms feature where a form can have multiple submit
buttons, and each submit button can have its own submit callback function named
{formname}_submit_{buttonname}. These functions are supposed to redirect at the end.
If they don't, due to a bug in the implementation of the global form message we currently show
an error message at the top of the form saying there was something wrong with the user's
submission. Instead, we should just let the form go through, and log a debug message for
the devs.

Change-Id: Ib0e9b7e2f58737c9c736bdd1c033bc020985a60b

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/3299
Committed: http://gitorious.org/mahara/mahara/commit/90a524681dce4e22b27e29a3d9d2007fb1004775
Submitter: Robert Lyon (<email address hidden>)
Branch: 1.9_STABLE

commit 90a524681dce4e22b27e29a3d9d2007fb1004775
Author: Aaron Wells <email address hidden>
Date: Thu Apr 24 14:28:24 2014 +1200

No need to show a different message when setting profile icon to the default

Bug 1311940: Also, a pieform button-level submit function is meant to redirect
at the end every time. And this one was not.

Change-Id: I7b71311df9aa29db9b5d7c892a0ee8b13442f7b3

Aaron Wells (u-aaronw)
Changed in mahara:
milestone: 1.10.0 → none
Aaron Wells (u-aaronw)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.