Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-25468

"Coding Error Detected" message saving Page resources when Description box empty

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.2
    • Component/s: Resource
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      The error message is this and only happens on some resources. It seems to occur when editing an existing Page resource.... On saving the Page this is logged (and the page doesn't save)

      Default exception handler: Coding error detected, it must be fixed by a programmer: Url parameters values can not be arrays! Debug: \n

      • line 393 of /lib/weblib.php: coding_exception thrown
        \n* line 446 of /lib/weblib.php: call to moodle_url->params()\n
      • line 3402 of /lib/navigationlib.php: call to moodle_url->param()\n
      • line 3211 of /lib/navigationlib.php: call to settings_navigation->generate_user_settings()\n
      • line 2579 of /lib/navigationlib.php: call to settings_navigation->load_user_settings()\n
      • line 583 of /lib/pagelib.php: call to settings_navigation->initialise()\n
      • line 599 of /lib/pagelib.php: call to moodle_page->magic_get_settingsnav()\n
      • line 134 of /blocks/settings/block_settings.php: call to moodle_page->__get()\n
      • line 279 of /blocks/moodleblock.class.php: call to block_settings->get_content()\n
      • line 232 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()\n
      • line 882 of /lib/blocklib.php: call to block_base->get_content_for_output()\n
      • line 934 of /lib/blocklib.php: call to block_manager->create_block_contents()\n
      • line 342 of /lib/blocklib.php: call to block_manager->ensure_content_created()\n
      • line 6 of /theme/formfactor/layout/general.php: call to block_manager->region_has_content()\n
      • line 627 of /lib/outputrenderers.php: call to include()\n
      • line 585 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()\n
      • line 587 of /course/modedit.php: call to core_renderer->header()\n, referer: http://my.moodle.site/course/modedit.php?update=4&return=1&sesskey=eHkDwqXdYZ

      I can reproduce this and will continue to investigate but someone might have some ideas or things to check

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            howardsmiller Howard Miller added a comment -

            The problem is that there is nothing in the Description box. Rather than displaying an error for that it falls over with this.,

            Show
            howardsmiller Howard Miller added a comment - The problem is that there is nothing in the Description box. Rather than displaying an error for that it falls over with this.,
            Hide
            howardsmiller Howard Miller added a comment -

            noting some similar sounding reports (fixed) I updated to 2010112400 but it makes no difference.

            Show
            howardsmiller Howard Miller added a comment - noting some similar sounding reports (fixed) I updated to 2010112400 but it makes no difference.
            Hide
            tsala Helen Foster added a comment -

            I just tried creating a new page resource on http://qa.moodle.net/ and obtained the same error message.

            Show
            tsala Helen Foster added a comment - I just tried creating a new page resource on http://qa.moodle.net/ and obtained the same error message.
            Hide
            tsala Helen Foster added a comment -

            The same error message occurs in MDL-25121 with a slightly different set-up.

            Show
            tsala Helen Foster added a comment - The same error message occurs in MDL-25121 with a slightly different set-up.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Well,

            problem here is that portfolio stuff is conflicting withe the module name ("page"). As far as we cannot change the module name, I think this is something to be fixed in the portfolio stuff both by:

            • let's use "exclusive" (unique) names for params used by portfolio, i.e. instead of "page", "config"... use "portfoliopage", "portfolioconfig"...
            • only looking for those parameters when the page is really one portfolio page and is suitable for being using them (/user/portfolio.php, /user/portfoliologs.php ...) and not always (current approach).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Well, problem here is that portfolio stuff is conflicting withe the module name ("page"). As far as we cannot change the module name, I think this is something to be fixed in the portfolio stuff both by: let's use "exclusive" (unique) names for params used by portfolio, i.e. instead of "page", "config"... use "portfoliopage", "portfolioconfig"... only looking for those parameters when the page is really one portfolio page and is suitable for being using them (/user/portfolio.php, /user/portfoliologs.php ...) and not always (current approach). Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Adding Penny in case we are missing something... for sure her expertise with portfolios will help. TIA!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Adding Penny in case we are missing something... for sure her expertise with portfolios will help. TIA!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Adding also to Sam... to ask if we have any other example of conditionally execution of code based in the request url along the navigation code or how this should be implemented (to resolve the second point above).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Adding also to Sam... to ask if we have any other example of conditionally execution of code based in the request url along the navigation code or how this should be implemented (to resolve the second point above). Ciao
            Hide
            dongsheng Dongsheng Cai added a comment -

            https://github.com/dongsheng/moodle/commit/11ba34de997bb92076ef102e0920dce78f970e17

            The above commit renamed page perpage config hide to portfolioxxxx, this fixed the problem, needs Sam's review.

            Show
            dongsheng Dongsheng Cai added a comment - https://github.com/dongsheng/moodle/commit/11ba34de997bb92076ef102e0920dce78f970e17 The above commit renamed page perpage config hide to portfolioxxxx, this fixed the problem, needs Sam's review.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi guys,
            I've just been chatting to Dongsheng about this now. To summarise I don't think that those optional param calls should be there or are needed at all.
            The links on the navigation shouldn't change based upon variations of a page you are viewing, i.e. portfolio > configure should always take you to the same page (the overview of your portfolios) whether you were on that page or configuring a particular portfolio instance. Keeping those links the same helps people understand the navigation and where they lead.

            So in regards to the params:

            1. hide : The hide param is used to toggle a user config option and I don't think there is any reason you would want to return to that page from the navigation.
            2. config, page, and perpage all change the state of the page you are viewing and as aboce I don't think these should influence the navigation, i.e. the transfer logs link in the navigation should always take you back to the initial page.

            So my +1 to do the following:

            1. Remove all portfolio optional params from navigationlib.php.
            2. user/portfolio.php
              1. Don't add hide to $PAGE->url, it doesn't need to be there as its not a page you will return to.
              2. If config is used as a param use navigation_node::override_active_url and give it a URL without config set so that the navigation still finds the correct node.
            3. user/portfoliologs.php use navigation_node::override_active_url and give it a URL without page or perpage set.

            The advantage of doing it this was is a) we don't need to rename those params so we won't break places we haven't thought of b) We remove the optional param calls from navigation lib that I really don't like being there and c) we keep the navigation links predictable and we won't go off surprising our users by changing where links lead.

            How does that sound to everyone?

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi guys, I've just been chatting to Dongsheng about this now. To summarise I don't think that those optional param calls should be there or are needed at all. The links on the navigation shouldn't change based upon variations of a page you are viewing, i.e. portfolio > configure should always take you to the same page (the overview of your portfolios) whether you were on that page or configuring a particular portfolio instance. Keeping those links the same helps people understand the navigation and where they lead. So in regards to the params: hide : The hide param is used to toggle a user config option and I don't think there is any reason you would want to return to that page from the navigation. config, page, and perpage all change the state of the page you are viewing and as aboce I don't think these should influence the navigation, i.e. the transfer logs link in the navigation should always take you back to the initial page. So my +1 to do the following: Remove all portfolio optional params from navigationlib.php. user/portfolio.php Don't add hide to $PAGE->url, it doesn't need to be there as its not a page you will return to. If config is used as a param use navigation_node::override_active_url and give it a URL without config set so that the navigation still finds the correct node. user/portfoliologs.php use navigation_node::override_active_url and give it a URL without page or perpage set. The advantage of doing it this was is a) we don't need to rename those params so we won't break places we haven't thought of b) We remove the optional param calls from navigation lib that I really don't like being there and c) we keep the navigation links predictable and we won't go off surprising our users by changing where links lead. How does that sound to everyone? Cheers Sam
            Hide
            mjollnir Penny Leach added a comment -

            sounds good to me go go sam!

            Show
            mjollnir Penny Leach added a comment - sounds good to me go go sam!
            Hide
            dongsheng Dongsheng Cai added a comment -

            Thanks Sam

            Can you please have a look at this commit.

            Regards,
            Dognsheng Cai

            Show
            dongsheng Dongsheng Cai added a comment - Thanks Sam Can you please have a look at this commit. Regards, Dognsheng Cai
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Dongsheng,

            The changes overall look good however the changes to user/portfoliologs.php aren't quite right yet.
            You've removed the page and limit params from the page URL and have then called navigation_node::override_active_url with a URL that is now identical to $PAGE->url.
            I think the best thing to do is to still add those params to the URL that is going to be given to $PAGE->set_url, and to call override_active_url with a URL that doesn't contain those two params:
            e.g.

            // Base URL
            $url = new moodle_url('/user/portfoliologs.php', array('courseid'=>$courseid));
            // Set the active URL now before we add page and limit params so that we find
            // the correct node on the navigation
            navigation_node::override_active_url($url);
            // Add the page and limit params to complete the current URL
            if ($page !== 0) {
                $url->param('page', $page);
            }
            if ($perpage !== 0) {
                $url->param('perpage', $perpage);
            }
            // Make $PAGE->url the URL of the current page in case an incidental action
            // such as setting a user preference causes the page to refresh
            $PAGE->set_url($url);

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Dongsheng, The changes overall look good however the changes to user/portfoliologs.php aren't quite right yet. You've removed the page and limit params from the page URL and have then called navigation_node::override_active_url with a URL that is now identical to $PAGE->url. I think the best thing to do is to still add those params to the URL that is going to be given to $PAGE->set_url, and to call override_active_url with a URL that doesn't contain those two params: e.g. // Base URL $url = new moodle_url('/user/portfoliologs.php', array('courseid'=>$courseid)); // Set the active URL now before we add page and limit params so that we find // the correct node on the navigation navigation_node::override_active_url($url); // Add the page and limit params to complete the current URL if ($page !== 0) { $url->param('page', $page); } if ($perpage !== 0) { $url->param('perpage', $perpage); } // Make $PAGE->url the URL of the current page in case an incidental action // such as setting a user preference causes the page to refresh $PAGE->set_url($url); Cheers Sam
            Hide
            dongsheng Dongsheng Cai added a comment -

            Thanks, Sam.

            I just added page and perpage to $url back, and submitted the pull request PULL-243.

            Regards,
            Dongsheng

            Show
            dongsheng Dongsheng Cai added a comment - Thanks, Sam. I just added page and perpage to $url back, and submitted the pull request PULL-243. Regards, Dongsheng
            Hide
            tsala Helen Foster added a comment -

            Fix included in the weekly 2.0.1+. Thanks everyone.

            Show
            tsala Helen Foster added a comment - Fix included in the weekly 2.0.1+. Thanks everyone.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  21/Feb/11