Thanks for reporting this.

I've put that on the backlog.

In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

@Ankit, I thought I might have a bash at fixing this.

Looking at mod/book/locallib.php I see that the block is a 'false' block like the one found in mydashboard/mypubilc and as such is not a real block although it can look and work in a similar way.

That said, looking at this section for the TOC would it be possible to redirect the first region available to set something like...

$firstregion = reset($defaultregion->regions);

...in the following code like so...?
 

/**

 * Add the book TOC sticky block to the 1st default region available

 *

 * @param array $chapters

 * @param stdClass $chapter

 * @param stdClass $book

 * @param stdClass $cm

 * @param bool $edit

 */

function book_add_fake_block($chapters, $chapter, $book, $cm, $edit) {

    global $OUTPUT, $PAGE;

    $toc = book_get_toc($chapters, $chapter, $book, $cm, $edit, 0);

    $bc = new block_contents();

    $bc->title = get_string('toc', 'mod_book');

    $bc->attributes['class'] = 'block block_book_toc';

    $bc->content = $toc;

$defaultregion = $THEME->layouts->get_defaultregion();    

$regions = $PAGE->blocks->get_regions();

$firstregion = reset($defaultregion->$regions);

    $PAGE->blocks->add_fake_block($bc, $firstregion);

}

Gareth, I have just added you to this BUG issue hoping you might be able to offer guidance.

No worries Mary, I'll have a look after Educating Yorkshire.

Hi all, I edited the description so that the link did not automatically put you on a reply page.

Ok, given http://php.net/manual/en/function.reset.php, try this:

...

    $defaultregion = $PAGE->blocks->get_default_region();

    $PAGE->blocks->add_fake_block($bc, $defaultregion);

}

given in '/lib/blocklib.php':

public function add_fake_block($bc, $region) {

        $this->page->initialise_theme_and_output();

        if (!$this->is_known_region($region)) {

            $region = $this->get_default_region();

        }

        if (array_key_exists($region, $this->visibleblockcontent)) {

            throw new coding_exception('block_manager has already prepared the blocks in region ' .

                    $region . 'for output. It is too late to add a fake block.');

        }

        $this->extracontent[$region][] = $bc;

    }

and

    public function get_default_region() {

        $this->page->initialise_theme_and_output();

        return $this->defaultregion;

    }

In fact, given the code, this:

...

    $PAGE->blocks->add_fake_block($bc, 'fluffy-bunny-feet');

}

should also work 99.9% of the time .

Cheers,

Gareth

OK...adding

$PAGE->blocks->add_fake_block($bc, 'fluffy-bunny-feet');

}

Dear Mary Evans,

You're havin a laugh lass! Are you serious about implementing:

$PAGE->blocks->add_fake_block($bc, 'fluffy-bunny-feet');

??

So a '-10' from me for that and a '+10' to try this:

...

    $defaultregion = $PAGE->blocks->get_default_region();

    $PAGE->blocks->add_fake_block($bc, $defaultregion);

}

Cheers,

Gareth

Can you Peer Review this now?

Dear Mary Evans, Give me a mo, I'll see what I can do, been beavering away on CONTRIB-3240.

Is OK...no hurry.

Works as implemented.

To test, in M2.6 I:

1. Created a book.
2. Selected the 'Clean' theme.
3. Temporarily changed the 'incourse' layout to a default region of 'side-post' in '/theme/bootstrapbase/config.php'.
4. With editing on on the book page added a 'Logged in user' block which defaulted to 'side-post'.
5. Turned editing off, observed that the TOC was still in 'side-pre'.
6. Applied the patch.
7. Refreshed the page and observed that the TOC was in 'side-post'.

Works as implemented, see comment above.

For testing I don't think you need a 'Purge caches' as 'config.php' is a server side entity and not cached at the client end, therefore any change happens on page refresh.

Thanks Gareth for Peer Review and help with this. Your understanding of Moodle functionality is amazing. Mind you it's your line of work, as dressmaking is really mine, but that's another story!

I also forgot to set this for IR and so I think have missed the slot for this week...I could kick myself!

I was also messing about with the settings and put it as an Improvement, which it is and it isn't, it's really a bug and as such needs back-porting so will add more branches to do this.

Thanks again for your invaluable help.
Cheers
Mary

Dear Mary Evans,

No worries , glad I could help.

Cheers,

Gareth

I am worried about this proposed change.

Everywhere else that we add fake blocks to pages (e.g. quiz, lesson) we add them to the first listed block region. Now, you suddenly want to make it inconsistent in one place for no good reason.

Well, OK, not for no good reason, but because that will look better in your particular theme, and you don't seem to care whether that will break things for everyone else's themes.

Note that the order in which you specify the regions in your theme's conig.php has no significance, except that the first region is the one used for any fake blocks. Therefore, if you want your fake blocks to appear in side-post, then just list that first. I think you should use that work-around in your themes for now, and this change should not be integrated.

In the longer term, I think we should not rely on the order of the array here (which is not very intuitive). Instead, we should introduce a new setting, a bit like the existing 'defaultregion' but called 'fakeblockregion' to control this explicitly. To maintain backwards compatibility, we can make fakeblockregion default to the first named region.

So, in summary, -1 from me, if you are asking.

Hi Tim Hunt,

The patch applies to all themes, Clean is just a defined example upon which to pin a test step on.

The 'defaultregion' is mandatory when having regions in a theme's config.php file -> http://docs.moodle.org/dev/Themes_overview#Layouts. And therefore has a more precise
intent from the theme's designer as to where blocks should go when their region has not been specified. It is easier to be clear on what is happening rather than saying that the order needs to be set in the array.

I think having another setting like 'fakeblockregion' is silly as introduces another configuration setting to cope with.

If everywhere else is less precise in stating where fake blocks should be placed, like lesson, quiz etc. then they need fixing and pinning down too.

So I would say that the 'good reason' is that this implements the theme designer's intent of 'what happens when the block region has not been specified that the default region given should be used'. Which marries up with the documentation line of "is the default location when adding new blocks" and therefore as fake blocks are 'new blocks' on the page, then this patch fixes the fact that they were not being added as they should have been.

Has everywhere else done (lesson, quiz... etc.) done the first item in the array solution because the first incarnation of the code did it that way and we have bug propagation? Just because everywhere else does it one way does not mean that this way is wrong because it would be inconsistent, it means a whole collective was wrong in the first place and needs correcting.

Cheers,

Gareth

Yes, I looked at the patch, and can see that it affects all themes, whether the creator of the theme wants the change or not. I do not think this is a good thing.

defaultregion does indeed have a precisely defined meaning. "defaultregion is the default location when adding new blocks" You give a link to that page, but then you do not accurately quote the definition of defaultregion given there. This does not strengthen your reasoning.

Changing that to include "also where other things like the book navigation gets added" just makes it less clear. New block should probably be added somewhere out of the way. Important parts of the UI like the quiz, book or lesson navigation need to be easy for users to see. This is something different. Theme designer should be able to specify it separately.

I think the first place to do this was the quiz. I implemented it this way for a good reason. Other people (rightly) copied that to maintain consistency. If you don't believe me, check git history.

Dear Tim Hunt,

I believe you without checking git history.

The definition of 'defaultregion' (author, date) being "is the default location when adding new blocks" is a direct copy (Ctrl-C -> Ctrl-V) from the page http://docs.moodle.org/dev/Themes_overview#Layouts. So I 'have' accurately quoted the definition (unless Windows 7 has some sort of clipboard English mangling function) and to strengthen my reasoning, it is an accurate quote and in Harvard style would be:

(author - need to search the history) (date from log file), Theme Overview - Layouts, available from http://docs.moodle.org/dev/Themes_overview#Layouts (accessed 9th September 2013).

Good enough reference?

I'm not saying that "Other people (rightly) copied that to maintain consistency" is wrong, it is just my view that you were wrong in the first place as I consider that new blocks should be added in a place that is obvious so that it is clear to the user that they are new and a decision needs to be made about them. Rather than hiding them away trying to pretend they don't exist.

I do agree that "also where other things like the book navigation gets added" is silly, which is why the other places need fixing before it needs to be written.

I don't think this is particularly a theme decision rather than a marriage of the mod designer having the fake block in the region where it should be which matches up with the description imparted to the theme designer about new blocks. So, I don't think it's an issue of whether the theme designer wants the change or not rather that it does not behave at the moment as it should and needs fixing.

Cheers,

Gareth

If what you are saying is correct Tim, then why bother having a default region at all for Adding Blocks? Why not just let them be added to the first available block location like a fake block gets added?

Lots of people do not like the way the QUIZ Question bank, which is a fake block, takes up space on the right in a 2 column page which turns it into a three column page, but have to put up with it because YOU want it there. And yet YOU dare to question my judgement, as though you assume I have not thought this out, did YOU really think out the location of the 'Question Bank' logically?

The way a block lands on a page, is as far as I can see, just following Moodle logic which requires a default region to be set. A default region becomes Theme specific by nature of the $THEME->layouts configuration. A default region dictates where blocks should land on a page, and for this reason, and this reason alone, the TOC is more likely to land in the right place then the wrong one...in a theme.

Now we are talking...'default_block_region', 'fake_block_region' and a much needed 'custom_black_region' are ideas I would willingly subscribe to if it meant that I could add blocks where I want them in a theme.

Hello Tim and others,

I am the person who reported this problem. And while I am a newbie, I think this "fake_block" placement issue is a real issue, not just "suddenly want(ing) to make it inconsistent in one place for no good reason." as you suggested.

It doesn't just affect one theme. And it doesn't just affect TOC in Book. It also affects all Question Modules. And who knows what other areas. I tried to file on a bug on this in Core Questions when I first noticed it. Dan Marsden rejected it because I only referenced ddMarker, which is a third party plugin (understandable). But Jamie Pratt indicated this is true in ALL question modules.

If you check you will see I was waiting for a response from you before filing the bug from respect for Jean-Michele. And I see your response. But your response doesn't address the fundamental issue. From my perspective, simply saying "neither choice will please everybody all of the time" is a false choice. If the location can change, then a person who wants it in the "first defined block region" can have it there. And the person who doesn't can move it. Everyone can be satisfied. But if it defaults to the first block, then all choice is removed.

My argument in favor of making this, or a similar, change involves three simple issues(newbies often see things simply):

First, this is a bug because it interferes with operations that a user might rightly expect to be able to do (change location of a TOC or a Question Navigation Menu) when a solution to permit this action exists AND it is achievable without a detriment to other users who do not wish to move it (see link at http://en.wikipedia.org/wiki/Software_bug).

Second, you can actually move the TOC or the Navigation blocks while in edit mode. But when you exist edit mode they revert back. To me, this indicates a BUG.

Third, you have suggested this is related to only a single theme. But that is not true. It emerges in the Essential theme. But I have struggled with placement of the TOC and Question Navigation Menu since I started using Moodle, about 1 year ago. That the TOC or Question Navigation Menu should be confined to a right hand column has no sense to it. What parameter other than inability to do otherwise in the past would confine this menu to one place or another. It is purely a programming convenience that may have outlived its time.

Now, again, as a newbie, I understand (somewhat) that Tim might be proposing a broader integration. And I can see how that would work (somewhat). But to suggest this is being done for a single theme, or is not worth doing basically is, in my humble opinion, inacurate. Whenever increased flexibility is possible without undermining basic function, then it seems it should be done. I can think of no exceptions.

Rod

Mary Evans the fact that you can seem some merit in my 'fake_block_region' suggestion gives me some confidence that I am not completely crazy. Phew! However, I don't yet see what you mean by 'custom_bl[o]ck_region'. Would you like to explain why that is needed?

Anyway, I think the general principal of giving theme designers more control is right. With a secondary principal of not increasing the work-load for theme designers. So, add new settings, but make them optional with sensible defaults.

------

The next thing I don't understand is: "space on the right in a 2 column page which turns it into a three column page". If the theme layout only has two columns (main content, and one block region) then the quiz will add the block to that one block region and the layout will stay two-columns.

Are you talking about the scenario where the admin chooses a three-colum theme, but then moves all the blocks to one block region to produce a two-column effect in a three column theme?

In that case, none of the proposals discussed here will work. Consider a three-colum theme with regions pre- (listed first) and post- (the defaults). If that admin moves all the block to pre-, then the current Moodle code works. If they move all the blocks to post-, then the proposed new code works. No proposal, even fake_block_region set by the them designer, handles both cases simultaneously.

------

So, anyway, yes I did think out the positioning of the question navigation block. Indeed, I probably started by using default block region, and found that it worked badly in most of the three-column core themes, so then I changed it. In a typical three-column Moodle theme, defaultregion is on the right (before MDL-32879 changed it in 2.3) and the quiz navigation should be on the left. Even though defaultregion changed, it still feels wrong to me to use that to position the quiz nav. In fact, because it has changed in the past, I am more reluctant to use it for an important part of the quiz UI.

But, I acknowledge that the way we control the positioning of the quiz nav block is not ideal, and this does cause real problems in some themes. That is why I proposed what I hope is a better way.

------

In summary, should we start a thread in the Themes forum, to see what people there think about the proposal to have an explicit fake_block_region setting?

------

(Gareth J Barnard your link to the docs was a perfectly good reference. The problem was with your claim that it said "where blocks should go when their region has not been specified" when actually it says "the default location when adding new blocks".)

Dear Tim Hunt,

I must apologise as I did not clearly explain myself. When I said about marriage and "is the default location when adding new blocks" is exactly the point, they are 'new blocks' that have been added to the page. As the method 'add_fake_blocks()' is 'adding' a new block to the page then it within the meaning of the method that it fulfills the intent that it is the place where the theme designer would wish new blocks to be added when they are an addition to the a page when the block location has not been specified. I would not consider that in this instance that the first element in the array is a specific enough statement of intent as to be clear in matching the definition within the documentation.

Cheers,

Gareth

Dear Mary Evans,

This "And yet YOU dare to question my judgement, as though you assume I have not thought this out, did YOU really think out the location of the 'Question Bank' logically?" I have to frame

Cheers,

Gareth

I feel the need to go on another long bus ride like Tim's mum! LOL

I've had a quick scan of the M2.5.2 code and it looks like only the quiz and lesson needs changing too. The calendar block explicitly states to use 'side-post', which perhaps might be an issue if there is no 'side-post'?

The Calendar is a pain to style. Can you check what page_layout it uses? Side post used to be the default block region so that was probably set like that on perpose.

Hi Mary Evans,

'/calendar/view.php' line 113 has:

$PAGE->set_pagelayout('standard');

But 'preferences.php' uses both 'standard' and 'admin'. And 'event.php' uses 'standard'.

Cheers,

Gareth

Calendar has the code:

    public function add_pretend_calendar_block(block_contents $bc, $pos=BLOCK_POS_RIGHT) {

        $this->page->blocks->add_fake_block($bc, $pos);

    }

in '/calendar/renderer.php' where 'BLOCK_POS_RIGHT' is set at the top of '/lib/blocklib.php':

define('BLOCK_POS_LEFT',  'side-pre');

define('BLOCK_POS_RIGHT', 'side-post');

And the method 'add_pretend_calendar_block()' is used only in '/calendar/lib.php' in the method 'add_sidecalendar_blocks()':

    public function add_sidecalendar_blocks(core_calendar_renderer $renderer, $showfilters=false, $view=null) {

        if ($showfilters) {

            $filters = new block_contents();

            $filters->content = $renderer->fake_block_filters($this->courseid, $this->day, $this->month, $this->year, $view, $this->courses);

            $filters->footer = '';

            $filters->title = get_string('eventskey', 'calendar');

            $renderer->add_pretend_calendar_block($filters, BLOCK_POS_RIGHT);

        }

        $block = new block_contents;

        $block->content = $renderer->fake_block_threemonths($this);

        $block->footer = '';

        $block->title = get_string('monthlyview', 'calendar');

        $renderer->add_pretend_calendar_block($block, BLOCK_POS_RIGHT);

    }

Cheers,

Gareth

P.S. Install Netbeans (https://netbeans.org/downloads/index.html) the PHP versions have some wicked ability to find stuff.

@Tim: I suppose we could start a forum discussion to thrash this out, but I do find that sort of thing so utterly tedious.

In fact I think if we were to ask a question in the Forums if Add Block was a drop-down menu list in the header, in much the same way as the Lang menu sits, where you could choose to not only 'Add-a-Block' but also choose which side it went to, when editing is enabled, then I think the majority of Moodlers would say "YES"! That way we could dispense with default block region once and for all, and the fake block could be set as you suggested with your 'fake-block-region', and I and lots of other designers could make different areas into 'custom-block-regions' that would be instantly recognised in Moodle without having to comply with overly complicated procedures for adding a custom block region.

If 'fake-block-regions' is the way to go then to be honest, I don't really like the word 'fake' as they are blocks, just not from the 'blocks' folder, but they are real. Perhaps something like 'function-block-region' as in the design concept of 'form and function'. Just a thought.

+1 for 'Add block' being on the menu.

This looks interesting as it appears Martin has also spotted something ODD about the Quiz and David Mudrak mentions the Book TOC...so looks like this could be a fix in Core...or is this just in the community site?

https://tracker.moodle.org/browse/MDLSITE-2421

@Tim it looks like this idea of using the default region for fake-blocks is one that has been deployed if we are to assume such from the fix applied in MDLSITE-2421

Just added David Mudrak as I thought he might be able to shed more light on this topic.

Yes, I discussed MDLSITE-2421 with Martin Dougiamas as you can see there. They ended up using the work-around I suggested: re-order the layouts array.

fake-blocks may not be the best name. They are fake (not real blocks) in the sense that

• There is no corresponding row in the block_instances table.
• There is no block_quiznavigation plug-in.

Thus, they are different from real blocks. If you can think of a name that is sufficiently better than fake to justify changing the API, then please suggest it.

(Note that, when the OU first implemented the quiz navigation block as a custom hack in Moodle 1.x, we did it as a real block, and that was a total nightmare. You had to make sure that there was exactly one instance of the block for each quiz, and that kept going wrong. Fake blocks work much, much better.)

In David's code he added $PAGE->blocks->add_fake_block($navbc, $PAGE->blocks->get_default_region()); here...
 

 // Arrange for the navigation to be displayed in the first region on the page.

 $navbc = $attemptobj->get_navigation_panel($output, 'quiz_attempt_nav_panel', $page);

-$regions = $PAGE->blocks->get_regions();

-$PAGE->blocks->add_fake_block($navbc, reset($regions));

+$PAGE->blocks->add_fake_block($navbc, $PAGE->blocks->get_default_region());

Which is what I am proposing in this fix...isn't it?

Yes

For the record, IIRC the Lesson module does somewhat more clever logic that it does not put the fake block into a region if the fake block would be the only one there. That sounded to me as a good thing to do.

David's recall is not accurate. See lesson_add_fake_blocks in mod/lesson/locallib.php. That adds some blocks to the first region, because they are important, and should be prominent, and others to default region that should be more out-of-the way. I assume that was done before Mary changed default region in 2.3 to be the same as the first region in most core themes.

Note that adding fake blocks only to non-empty regions does not work. The quiz attempt page often has no real block on it.

Some thoughts:

1) I really find the use of get_default_region() far better than current arbitrary first/reset implementation, mainly coz it guarantees the region is a blocks one. Just that makes it better.

2) Still it continues being an arbitrary implementation and it should be something configurable, from the activity or whatever.

3) We should get all them (book, lesson, quiz) and try to make all them to have the same behaviour. Configurable, non-movable in edit mode… whatever. Surely 2.6 only. Would be great to have an issue for such "normalization" to happen.

4) I think this is ok as is. It won't change much and will allow themers/devs/users to compare "fixed" book behavior vs. quiz/lesson one.

Integrated (24, 25 & master), thanks!

Thank you, Eloy, that sounds very encouraging.

Works as described.
Passing, thanks.

Thanks Ankit

Yarrr me 'arties, good job done. Yer code 'as landed and the weeklies ave been released with your contributions in tow.
The brethren thank ya for yer 'ard work and if there'd been treasure to ave ya would ave got yer cut.

Thanks for the effort everyone, another successful weekly release has been rolled.
Please keep in mind code freeze is just around the corner now, get your new features and improvements in ASAP.

Many thanks
Sam

Hi,

I've the same problem as it is report in the discussion here: https://moodle.org/mod/forum/discuss.php?d=236847#p1028595
In the discussion there is a link to this tracker entry.

My problem - I've tried to replace the TOC of the book module:

• turn editing mode in book module on
• replace TOC from upper left column to upper right column (change is visible)
• turn editing mode off and the TOC will be located at origin side

Tested in Moodle 2.5.4 and Moodle 2.6.1. Same results.

Am I doing something wrong? Perhaps we can reopen this issue?

Hi Gerald Hartwig,

Have you tried changing the 'defaultregion' in the 'config.php' file as stated in the testing instructions?

Cheers,

Gareth