Problems saving view layout - no option selected by default

Bug #1307760 reported by Robert Lyon
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Robert Lyon
1.9
Fix Released
High
Robert Lyon

Bug Description

When I make a new page and click on the layout tab and then click save the system throws errors. This is due to no layout option being selected by default.

If I selected 'advanced' or 'basic' sections then the 3 column layout get selected - but this is done in a less than optimal way as the value for the layout is hardcoded in the js so it is expecting that the layout option for the 3 column layout will have the id = 5, but this might not always be the case. We really need to find the default layout with php and pass the id to the js.

Also in th js code there are js variables like $selected, which are written like they want to get the value from php.

It would be better for the variables:
- set by js to be written as 'var myvar ='
- set by jquery/mochikit to be written as '$()' or '$j()' (where appropriate)
- and have the php only be used on the right hand side of the sentance eg have 'var myvar = $phpvar'

Tags: regression
Robert Lyon (robertl-9)
Changed in mahara:
assignee: nobody → Robert Lyon (robertl-9)
status: Confirmed → In Progress
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/3263

Revision history for this message
Piroska Pulliainen (piroska-pulliainen-z) wrote :

There is also a bug with the function (viewlayout_validate) throwing the error.

Missing argument 2 for Pieform::set_error(), called in .../htdocs/view/layout.php on line 180
Line 180 looks like this:
$form->set_error('invalidlayout');
The form name is missing ('layoutselect') and the error message string could be 'invalidlayoutselection'.

$string['invalidlayoutselection'] = 'You tried to select a layout that doesn\'t exist.';

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

Hi Piroska,

Ah good point - I've updated the patch to also fix up the validation error.

Cheers

Robert

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

Reviewed: https://reviews.mahara.org/3263
Committed: http://gitorious.org/mahara/mahara/commit/a4ba6cacf022fafa583cfdbb37773aed92641005
Submitter: Son Nguyen (<email address hidden>)
Branch: master

commit a4ba6cacf022fafa583cfdbb37773aed92641005
Author: Robert Lyon <email address hidden>
Date: Tue Apr 15 15:39:59 2014 +1200

Set default layout columns/widths hardcoded once (Bug #1307760)

And making sure that it exists in the db before using it.
This allows the fetching of the id of the default column widths to be
used in view layout page.

Also tidied up the js for the view layout page so it uses the value
from php rather than hardcoded string.

Also removed duplicate 'if (empty(self::$layoutcolumns)) { }'
statement from lib.view.php

Change-Id: Icf1b4292a256c5ff6d748c9e6046d6c71ea298b2
Signed-off-by: Robert Lyon <email address hidden>

Robert Lyon (robertl-9)
Changed in mahara:
milestone: 1.9.1 → 1.10.0
status: In Progress → Fix Committed
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/3354

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

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

commit 9aff53a4946e2570d486793169a1a0b4a0a5b45c
Author: Robert Lyon <email address hidden>
Date: Tue Apr 15 15:39:59 2014 +1200

Set default layout columns/widths hardcoded once (Bug #1307760)

And making sure that it exists in the db before using it.
This allows the fetching of the id of the default column widths to be
used in view layout page.

Also tidied up the js for the view layout page so it uses the value
from php rather than hardcoded string.

Also removed duplicate 'if (empty(self::$layoutcolumns)) { }'
statement from lib.view.php

Change-Id: Icf1b4292a256c5ff6d748c9e6046d6c71ea298b2
Signed-off-by: Robert Lyon <email address hidden>

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.