The fix is easy (I did it years ago)!

I could probably post a patch if I get some time, but prob not til after the Moot at Open U next week

This behavior caused havoc among our users, and I was forced to turn frames back on to get around it in the short term.

We mitigated this problem by changing the default from "same window" to "new window". This does not solve the problem but makes it more manageable as users are at least able to continue to navigate the site.

[Save and return to course] [Save and display]

The only way I know of to have 2 submit buttons that do different things it to compare the value in the returned by the button, but I was curious if this would be cross language reliable. IE:

In mod/resource/mod_form.php, just before $this->add_action_buttons(); you could add:
$mform->addElement('submit', 'submitbutton', get_string(savechangesandreturn));

The problem later on is you have to do:

if ($fromform->submitbutton == get_string(savechanges)) {
//save changes and preview
} else {
//save changes and return to course.
}

The question is, can you reliably compare the submitted value for the button against the string in get_string. I know it should work for latin languages, but I'm not clear if it will work in other languages reliably.

Is there some other way of doing this (that doesn't completely rely on javascript) that Im missing?

I think we could extend add_action_buttons in moodleform_mod in course/moodleform_mod.php to add that extra submit button.

$mform->addElement('submit', 'submitbutton2', get_string(savechangesandreturn));

Note the different element name (submitbutton2). IIRC only the value for the submit button you clicked gets sent (ie submitbutton OR submitbutton2)

And then in modedit.php where it has

redirect("$CFG->wwwroot/mod/$module->name/view.php?id=$fromform->coursemodule");

make it check for the two different submit values and if !empty(submitbutton2) go to the course page instead.

Something like that. Go Nicolas.

Looks great. Nice job Nicolas!

Thanks for the fix, this will be a huge usability improvement for our faculty!

Will this be back-ported to the 1.8.x tree?

Very useful. Many thanks for the fix.

In course/modedit.php (Moodle1.8.4+), Line15, 16,
$submitbutton = optional_param('submitbutton', null, PARAM_ALPHANUM);
$submitbutton2 = optional_param('submitbutton2', null, PARAM_ALPHANUM);
These variables are going to return null, since "method" of <FORM> is POST.

Please try following codes;

// if ($submitbutton) {
if (!empty($fromform->submitbutton)) { redirect("$CFG->wwwroot/mod/$module->name/view.php?id=$fromform->coursemodule"); // } elseif ($submitbutton2) {
} elseif (!empty($fromform->submitbutton2)) { redirect("$CFG->wwwroot/course/view.php?id=$course->id"); // }
} else redirect("$CFG->wwwroot/course/view.php?id=$course->id"); // (ADD)
exit;

optional_param does accept input of type POST or GET.

This code would seem to mask a different problem and perhaps introduce new ones (like what happens when you go back in the browser). The existing code does work in other installations. And even if I am misunderstanding the issue, it can be simplified to just go to course/view.php if $fromform->submitbutton is not true.

I would suggest that this be kept open until there is consensus on this.

Note the cleaning of optional_params does and that this is not happening in the proposed code.

But both of $submitbutton and $submitbutton2 are null in Japanese environment.
I guess why both variables become null, the cause of this is $submitbutton/2 are clean_param() by PARAM_ALPHANUM. In japanese environment, value of $_POST['submitbutton'] is "???????????"(Non ALPHANUM characters!). Then, all characters are cleaned up -> null.

Maybe, it will be ok, by changing 3rd parameter of optional_param(); PARAM_ALPHANUM to PARAM_RAW.

Now I'm in home. I can not try this.
Next morning I will try and report here.

if ($submitbutton) { redirect("$CFG->wwwroot/mod/$module->name/view.php?id=$fromform->coursemodule"); } elseif ($submitbutton2) { redirect("$CFG->wwwroot/course/view.php?id=$course->id"); // }
} else redirect("$CFG->wwwroot/course/view.php?id=$course->id"); // (ADD)
exit;

I think that if-elseif-"else" needs for safety, too.

Can you please:

1) Remove the optional_param stuff from the top of modedit.php for $submitbutton and $submitbutton2 because they are no longer used.
2) Explicitly set the PARAM types for the submit buttons to be PARAM_RAW to make it work consistently with all languages (it's safe since we aren't actually using the value, just checking for non-null).

Actually, why are we checking the contents of the buttons at all ... can't we just check for isset($fromform->submitbutton) ?

Please confirm that the multilang issue is resolved.

I know its pedantic, but could the order of the buttons be reversed so that 'save and return to course' is the first one, i.e. the one on the left? For novice users, that is where they are most likely to click, not knowing the difference between them, and although I know the functions are written on them, I have also done a lot of training before and suspect that people will still do this.

I propose two ideas;
(1) Save and [Display] [return to course] : not so good...
(2) [Save and return] [x] Display

In second idea, [x] means check box. In default this is unchecked.

But I do agree with Matt that the course button should go on the left, it naturally fits with the idea of the breadcrumb navigation (and browser navigation) in that left is "go back" to the course and right is "go forward".

I've fixed this in 1.9.