Broken html error message for no-reply email setting

Bug #946880 reported by Kristina Hoeppner
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Low
Nigel Cunningham
1.10
Fix Released
Low
Unassigned
1.8
Fix Released
Low
Unassigned
1.9
Fix Released
Low
Unassigned
15.04
Fix Released
Low
Nigel Cunningham

Bug Description

error message when upgrading from 1.4 to 1.5:

The noreply address setting is either empty or has an invalid email address. Please check the configuration in the <a href="http://15stable/admin/site/options.php?fs=emailsettings">site options in the email settings</a>.

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

That warning works fine when you are in the site admin area, but it shows up as escaped HTML after the install.

Changed in mahara:
importance: Medium → Low
Changed in mahara:
milestone: 1.5.0 → none
tags: added: academy
Changed in mahara:
status: Triaged → In Progress
assignee: nobody → Christian Tuveve-Aiono (handlethesandal)
assignee: Christian Tuveve-Aiono (handlethesandal) → nobody
Changed in mahara:
assignee: nobody → Christian Tuveve-Aiono (handlethesandal)
Changed in mahara:
assignee: Christian Tuveve-Aiono (handlethesandal) → nobody
Changed in mahara:
status: In Progress → Triaged
Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

Attached is the error message on the first Mahara screen right after a fresh install of master.

Aaron Wells (u-aaronw)
tags: added: upgrades
Revision history for this message
Lucas Dima (lucdima) wrote :

The HTML of the warning message was escaped. The method that displays the messages has a default parameter for escape HTML as true.
The solution is to set to false the second parameter when that method is invoked:

Method definition on class Session.
public function add_error_msg($message, $escape=true) {

Method invocation on line 37 of file admin/upgrade.php
$SESSION->add_error_msg($w);
should be: $SESSION->add_error_msg($w,false);

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

What are the steps to replicate the original problem? I don't see any warning when I do a clean install. $cfg->noreplyaddress seems to get set to a sensible default by init.php if none is supplied.

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

I tried it using 1.7_STABLE and master.

Revision history for this message
Lucas Dima (lucdima) wrote :

Apparently the problem is actually only visible on upgrade, not on clean installs.
Looks like it's just how the HTML is displayed. According to the method add_error_msg, witch displays the error, the HTML code inside errors can be escaped or not with the second parameter set it to FALSE or TRUE. The error messages had HTML and the method invocation was set to escape HTML (The default option), giving as result HTML escaped, where it should not be escaped, it should be displayed as HTML.

Revision history for this message
Robert Lyon (robertl-9) wrote :

I see this error warning on fresh install if I have set the url to be something based on an entry in my /etc/hosts file eg:

 127.0.0.1 mahara-testing

and Apache set up for that so the url for the site is http://mahara-testing/ and therefore $cfg->noreplyaddress gets set to something bad like:

 noreply@mahara-testing

The fix $SESSION->add_error_msg($w,false); will get the href link to display correctly.
But should there be a link in the warning at this stage as one can't navigate to that page until they have set their default password and email first.

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

I tried navigating to the site by just using the link, was asked for a new password and email address and then was taken to the dashboard instead of the site settings. Thus, it would be better not to have a link there but change the wording. However, the same string is used on the admin overview page. Therefore, a new string would need to be created.

Changed in mahara:
assignee: nobody → Kristina Hoeppner (kris-hoeppner)
Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :
Revision history for this message
Robert Lyon (robertl-9) wrote :

There would need to be added a check to see if we are installing or not to display message with/without link.

Or we could just display: Administration -> Configure site -> Email settings as words rather than a link to indicate where user needs to go to edit

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

Since we have links everywhere else, a link would be great. If you aren't logged in yet, shouldn't the link redirect you to the login page form and then take you to the correct page?

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

Argh. That doesn't work because the site admin hasn't chosen the password or email yet. I would go for a new string as I said in comment #8 instead of reusing the one that a site admin sees on the site admin homepage when logged in. This new string would then not include a link as that doesn't take you anywhere properly unless this can be resolved.

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/3506

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

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

Revision history for this message
Nigel Cunningham (nigelc-g) wrote :

I have another suggestion, with a patch that I've just pushed.

The required fields form works by hijacking any link that the user has requested, so all we need to do after the user has successfully addressed the requirements is drop out of generating the form and let the normal execution for the requested URL continue. My patch accomplishes that by adding a small amount of code allowing this to happen. It relies on the fact that $form->build() always returns a string (the rendered HTML), and instead returns FALSE if we need to back out of the form generation.

I've successfully tested it by running an install on the URL http://mahara, which causes the warning about the noreply email address. I then clicked on the noreply link (fixed by the first patch of the two in the submission) and was taken to the site_options URL, overridden by the required fields form. After the required fields form was submitted with a valid admin password, the page loaded with the requested form for setting the noreply link.

At the point where I clicked on the noreply link, I also clicked on other links, opening them in other tabs. All tabs displayed the required fields form under the requested URL, as expected. After successful submission on one of the tabs, reloading all other tabs then displayed the expected page content.

Changed in mahara:
assignee: Kristina Hoeppner (kris-hoeppner) → Nigel Cunningham (nigelc-g)
status: Triaged → In Progress
milestone: none → 1.10.0
Aaron Wells (u-aaronw)
Changed in mahara:
milestone: 1.10.0 → 1.10.1
no longer affects: mahara/1.10
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/3506
Committed: http://gitorious.org/mahara/mahara/commit/cb26cb8a8d6f5c2e49475788d7f942dfb0686ac6
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit cb26cb8a8d6f5c2e49475788d7f942dfb0686ac6
Author: Nigel Cunningham <email address hidden>
Date: Thu Jul 24 10:02:09 2014 +1000

Fix html for no-reply email setting (Bug #946880)

Fix an issue wherein HTML in a warning about the no reply
email setting is displayed as plain text.

Change-Id: I8bdf8325db93dc3a2bea4e040effdb641909e8eb
Signed-off-by: Nigel Cunningham <email address hidden>

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

There were two patches for this bug. The first one, https://reviews.mahara.org/#/c/3506/, fixes the problem initially described.

The second one, https://reviews.mahara.org/#/c/3507/, fixes a related problem described in comment #15.

I've merged patch 3506, but found problems with patch 3507. So to make record-keeping easier, I've moved patch 3507 to a new bug 1391393.

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

Patch for "1.10_STABLE" branch: https://reviews.mahara.org/3962

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

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

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

Patch for "1.8_STABLE" branch: https://reviews.mahara.org/3964

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

Reviewed: https://reviews.mahara.org/3964
Committed: http://gitorious.org/mahara/mahara/commit/b3a168117a9cd1cdec7a115fd95b8b1a3127636c
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.8_STABLE

commit b3a168117a9cd1cdec7a115fd95b8b1a3127636c
Author: Nigel Cunningham <email address hidden>
Date: Thu Jul 24 10:02:09 2014 +1000

Fix html for no-reply email setting (Bug #946880)

Fix an issue wherein HTML in a warning about the no reply
email setting is displayed as plain text.

Change-Id: I8bdf8325db93dc3a2bea4e040effdb641909e8eb
Signed-off-by: Nigel Cunningham <email address hidden>

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

Reviewed: https://reviews.mahara.org/3962
Committed: http://gitorious.org/mahara/mahara/commit/981608927926ba22dd9246934515f5580adcee6f
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.10_STABLE

commit 981608927926ba22dd9246934515f5580adcee6f
Author: Nigel Cunningham <email address hidden>
Date: Thu Jul 24 10:02:09 2014 +1000

Fix html for no-reply email setting (Bug #946880)

Fix an issue wherein HTML in a warning about the no reply
email setting is displayed as plain text.

Change-Id: I8bdf8325db93dc3a2bea4e040effdb641909e8eb
Signed-off-by: Nigel Cunningham <email address hidden>

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

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

commit 1a83b282acb2696fcacd1c11c5d039eef0d7461e
Author: Nigel Cunningham <email address hidden>
Date: Thu Jul 24 10:02:09 2014 +1000

Fix form submission for no-reply email setting (Bug #946880)

After installation or upgrade, if the user clicks on a warning
about the no-reply email address and they also need to set up
required fields, the URL for the no-reply email address is
initially overridden by the required fields form. This patch
causes us to cleanly drop back out of the required fields form
code after the required fields form is successfully submitted,
letting the user then see the no-reply email address form.

This method of handling things will also work for any other
time the required fields form hijacks a URL - after the required
fields are set, the user will get the page they asked for.

Change-Id: I32aecaf898d02a572a5ab7b5c18bfaefc5607e41
Signed-off-by: Nigel Cunningham <email address hidden>

Robert Lyon (robertl-9)
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.

Duplicates of this bug

Other bug subscribers

Remote bug watches

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