CHAT HISTORY Eloy <--> David from 17 Jun 2011

(01:31:31) Eloy: crazy, I get the emoticons button in the editor when editing course->summary
(01:31:43) Eloy: but not if editing one section
(01:32:06) mudrd8mz@jabber.cz/glum: wait - it sounds to me
(01:32:36) Eloy: is related with the filter being enabled?
(01:33:00) mudrd8mz@jabber.cz/glum: oh sure - filter ,must be enabled in the context to have the button displayed
(01:33:15) Eloy: I had the filter disabled @ course context... and course->summary continued showing the button.
(01:33:35) Eloy: now i've enabled the filter and yes, i get the button in section
(01:33:43) mudrd8mz@jabber.cz/glum: course->summary is maybe not seeing as course context level?
(01:33:52) Eloy: lol, sounds silly
(01:33:54) Eloy: :-)
(01:33:55) mudrd8mz@jabber.cz/glum: as it is displayed at front page too
(01:34:19) mudrd8mz@jabber.cz/glum: it is a border/envelope. who knows
(01:34:22) Eloy: it must know the context, for storing images and so on
(01:34:44) Eloy: disabling the filter again...
(01:34:46) mudrd8mz@jabber.cz/glum: ah, right. silly me
(01:35:20) Eloy: disapeared in section... ok
(01:35:39) Eloy: continues in course->summary...wrong
(01:36:12) Eloy: disabling @ system
(01:37:14) Eloy: disabled now in the editor
(01:37:29) Eloy: so really it's ignoring the course context, strange
(01:38:30) Eloy: disabled @ system but enabled @ course continues not showing it, so really it's using system there or so 
(01:39:03) Eloy: haha, the editor does not show it, but front-page, list of courses show the icon.
(01:39:17) Eloy: so i guess it's the editor the only ignoring the context.
(01:39:22) Eloy: the filter seems to work ok
(01:39:47) mudrd8mz@jabber.cz/glum: I am just looking for when the condition on the context is
(01:39:55) mudrd8mz@jabber.cz/glum: I may  be wring
(01:39:59) mudrd8mz@jabber.cz/glum: wrong
(01:42:08) mudrd8mz@jabber.cz/glum: is the button missing completely?
(01:42:13) Eloy: yes
(01:42:26) mudrd8mz@jabber.cz/glum: and some JS debugging? error thrown or so?
(01:42:34) Eloy: that's the reason i asked originally, I din't saw the yellow face at all.
(01:42:43) Eloy: no, nothing... looking error logs, moment...
(01:43:12) mudrd8mz@jabber.cz/glum: I can't find how the link to the filter is implemented, if ever
(01:43:22) mudrd8mz@jabber.cz/glum: link = relationship
(01:43:53) Eloy: aha, perhaps the call in course/edit is the one passing the context and it decides to show or no the button?
(01:44:04) Eloy: no errors in logs either
(01:44:19) mudrd8mz@jabber.cz/glum: but the call in course/edit.php provides course context
(01:44:52) Eloy: lololol
(01:44:56) mudrd8mz@jabber.cz/glum: lib/editor/tinymce/lib.php
(01:45:01) Eloy: very first lines of dialog.php
(01:45:07) mudrd8mz@jabber.cz/glum: $xemoticon
(01:45:07) Eloy: $PAGE->set_context(get_system_context());
(01:45:11) mudrd8mz@jabber.cz/glum: nono
(01:45:16) mudrd8mz@jabber.cz/glum: that is correct IMHO
(01:45:20) Eloy: ah
(01:45:37) mudrd8mz@jabber.cz/glum: this is the very small window with the list of emoticons
(01:45:42) Eloy: ah
(01:45:53) mudrd8mz@jabber.cz/glum: $context = empty($options['context']) ? get_context_instance(CONTEXT_SYSTEM) : $options['context'];
(01:46:34) mudrd8mz@jabber.cz/glum: lib/editor/tinymce/lib.php is the place to debug
(01:46:52) Eloy: i get not the emoticons filter there
(01:47:07) Eloy: just this:

Array
(
    [filter/mediaplugin] => Array
        (
        )

)


(01:47:15) Eloy: and I've it enabled 100x100
(01:47:17) mudrd8mz@jabber.cz/glum: and is it correct?
(01:47:19) Eloy: @ course
(01:47:20) Eloy: nono
(01:47:26) mudrd8mz@jabber.cz/glum: and the $context is what?
(01:47:28) Eloy: i've the emoticons "On"
(01:47:58) Eloy: crap:

stdClass Object
(
    [id] => 1
    [contextlevel] => 10
    [instanceid] => 0
    [path] => /1
    [depth] => 1
)
(01:48:21) mudrd8mz@jabber.cz/glum: so $options['context'] is missing
(01:48:49) mudrd8mz@jabber.cz/glum: ah!
(01:48:51) Eloy: and sections set the correct context: 

stdClass Object
(
    [id] => 656
    [contextlevel] => 50
    [instanceid] => 9
    [path] => /1/3/656
    [depth] => 3
)

(01:48:55) mudrd8mz@jabber.cz/glum: of course it is
(01:48:57) mudrd8mz@jabber.cz/glum: course/edit.php
(01:49:01) Eloy: yes
(01:49:03) mudrd8mz@jabber.cz/glum: $editoroptions,
(01:49:14) Eloy: missing?
(01:49:32) Eloy: yeah
(01:50:06) mudrd8mz@jabber.cz/glum: that smells that the context must be passed twice to file_prepare_standard_editor()
(01:50:14) mudrd8mz@jabber.cz/glum: but maybe there is a reason for that?
(01:50:27) mudrd8mz@jabber.cz/glum: bah, no
(01:50:34) Eloy: looking editsection for compaison
(01:50:53) mudrd8mz@jabber.cz/glum: 'context'=>$context
(01:51:04) mudrd8mz@jabber.cz/glum: line 42
(01:51:42) Eloy: yeah, it's there, and missing in course
(01:51:45) Eloy: yay
(01:51:55) mudrd8mz@jabber.cz/glum: I believe that file_prepare_standard_editor() must set the $options
(01:52:09) mudrd8mz@jabber.cz/glum: from the $context attribute
(01:52:14) mudrd8mz@jabber.cz/glum: param
(01:55:03) Eloy: you mean array & $options and add the context there if passed ?
(01:55:42) mudrd8mz@jabber.cz/glum: I think that $options['context'] should be ignored and the passed $context should be used only
(01:55:58) mudrd8mz@jabber.cz/glum: so it would effectively do $options['context'] = $context;
(01:56:52) Eloy: it is ignored
(01:57:08) Eloy: it's the form element or so the one needing that.
(01:57:29) Eloy: but file_prepare_standard_editor() does not use options[context] at all
(01:57:54) mudrd8mz@jabber.cz/glum: but the code in lib/editor/tinymce/lib.php does
(01:58:21) mudrd8mz@jabber.cz/glum: the one that decides which filters are enabled
(01:59:24) mudrd8mz@jabber.cz/glum: alternatively, file_prepare_standard_editor() could merge both $options['context'] and $context, filling $options['context'] is it is missing and throwing exception if different contexts are passed
(02:00:09) mudrd8mz@jabber.cz/glum: as I say, it is bad that the caller must provide both $options['context'] and $context param
(02:00:12) Eloy: yes, i like that more
(02:01:11) Eloy: later we can decide to ban / debug calls originally containing options[context] but fill if missing is better for now
(02:01:17) Eloy: and warn if different
(02:01:59) mudrd8mz@jabber.cz/glum: just warn?
(02:02:07) Eloy: boom
(02:02:09) Eloy: excep
(02:02:13) Eloy: *tion
(02:02:14) mudrd8mz@jabber.cz/glum: yup
(02:02:22) mudrd8mz@jabber.cz/glum: that is a nice warning :-)
(02:03:55) Eloy: so:

1) file_prepare_standard_editor() will pass $options by reference
2) if options[context] exists and $context exists and are diferent => BOOM
3) If options[context] is missing and $context exists, fill the former with the later
4) If both are empty... debugging?
(02:05:52) Eloy: is so redundant... file_prepare_draft_area() passes both again.
(02:06:21) mudrd8mz@jabber.cz/glum: why be reference?
(02:06:37) Eloy: or outer code won't know
(02:06:49) Eloy: when called the editor I mean
(02:07:10) mudrd8mz@jabber.cz/glum: ?
(02:07:43) Eloy: the options [context] must be passed to the form isn't it?
(02:08:15) mudrd8mz@jabber.cz/glum: yes but I believe that a shallow copy is used so $options['context'] is the same object
(02:08:18) Eloy: so the $editoroptions (in course/edit.php) must have the [context] set 
(02:08:27) Eloy: is not object, is array
(02:08:39) Eloy: $editoroptions = array('maxfiles' => EDITOR_UNLIMIT.....
(02:08:57) mudrd8mz@jabber.cz/glum: sorry, by "pass by reference" did you mean &$options ?
(02:09:04) Eloy: in fact file_prepare_standard_editor() cast it to array
(02:09:13) Eloy: yes in the file_prepare_standard_editor() declaration
(02:09:29) mudrd8mz@jabber.cz/glum: yes and I believe there is no need for that is there?
(02:09:36) Eloy: yes for arrays
(02:09:40) Eloy: no for objects
(02:09:42) Eloy: afaik
(02:09:44) mudrd8mz@jabber.cz/glum: no
(02:09:49) Eloy: yes
(02:09:51) Eloy: :-)
(02:09:55) mudrd8mz@jabber.cz/glum: array of objects
(02:10:00) Eloy: ?
(02:10:03) mudrd8mz@jabber.cz/glum: passed to a function
(02:10:09) mudrd8mz@jabber.cz/glum: does not clone objects
(02:10:21) Eloy: they are not objects. it's one array of strings
(02:10:37) mudrd8mz@jabber.cz/glum: $options['context'] is not an object?
(02:10:58) Eloy: yes, but the array item does not exist!
(02:11:08) Eloy: the element!
(02:11:13) Eloy: when called
(02:11:30) mudrd8mz@jabber.cz/glum: and the $editoroptions is re-used by the caller?
(02:12:15) mudrd8mz@jabber.cz/glum: ah crap I see it now
(02:12:23) mudrd8mz@jabber.cz/glum: grrr
(02:12:25) mudrd8mz@jabber.cz/glum: it sucks
(02:12:46) mudrd8mz@jabber.cz/glum: this smells more than having to pass it twice
(02:13:46) Eloy: 
<?php

$arr = array('hello' => 'hello');

myfunction($arr);

var_dump($arr);

function myfunction($arr) {

    $bye = new stdClass();
    $arr['bye'] = $bye;
}
(02:14:10) Eloy: then, set & in the decalration
(02:14:16) mudrd8mz@jabber.cz/glum: yes yes I did not realize that the modified editor options is needed later for mform
(02:14:23) Eloy: ah, yes,
(02:14:23) mudrd8mz@jabber.cz/glum: in that case I think the best way might be to just warn developer that $options['context'] is missing if the $context is passed.
(02:14:35) Eloy: oki, perhaps the simpler way, yup
(02:14:44) Eloy: and then fix the missing @ course/edit.php
(02:14:47) mudrd8mz@jabber.cz/glum: see above ^^^ "it sucks..."
(02:15:00) Eloy: ah, i was writting the test, lol.
(02:15:24) Eloy: was not 100% sure lol, perhaps 5.3 had changes that
(02:15:29) Eloy: *changed.
(02:15:33) mudrd8mz@jabber.cz/glum: again :-)
(02:16:07) mudrd8mz@jabber.cz/glum: file_prepare_standard_editor() should not modify the options in this silent way
(02:16:11) Eloy: yeah
(02:16:24) mudrd8mz@jabber.cz/glum: it has bad API actually
(02:16:32) Eloy: horrible
(02:16:36) mudrd8mz@jabber.cz/glum: it should use $options['context'] only
(02:16:39) mudrd8mz@jabber.cz/glum: not $context
(02:17:01) mudrd8mz@jabber.cz/glum: then we would not have this issue
(02:17:03) Eloy: yup, passing twice is redundant, for sure one or the other is superfluos.
(02:18:01) Eloy: lol, later course_edit_form() sets its context ignoring options too.
(02:18:23) mudrd8mz@jabber.cz/glum: yeah, messy messy
(02:19:02) Eloy: but passes $editoroptions to the editor, that is the ultimate responsible for that.
(02:19:34) Eloy: anyway... where should we be debugging... in the editor element?
(02:20:00) mudrd8mz@jabber.cz/glum: so:

1) $context !== $options['context'] BOOM
2) missing one of them DEBUGGING_DEVELOPER
3) missing both OK
(02:20:03) mudrd8mz@jabber.cz/glum: ?
(02:20:46) Eloy: but where are you going to do so? in file_prepare_standard_editor() ?
(02:20:53) mudrd8mz@jabber.cz/glum: I would say so
(02:20:59) mudrd8mz@jabber.cz/glum: as soon as possible
(02:21:08) Eloy: what if it's one editor without support to files?
(02:22:04) Eloy: ah, but the $context does not go to the editor mform element, we cannot check there.
(02:22:06) Eloy: grrr
(02:22:10) mudrd8mz@jabber.cz/glum: wait
(02:22:26) mudrd8mz@jabber.cz/glum: I am not sure now that file_prepare_standard_editor() is the place
(02:22:43) mudrd8mz@jabber.cz/glum: when is that tinymce/lib.php called? and where from?
(02:22:57) Eloy: from the editor element for sure
(02:23:28) mudrd8mz@jabber.cz/glum: aha - but that one does not have the $context
(02:23:38) mudrd8mz@jabber.cz/glum: only options
(02:23:52) Eloy: lol, that's what i was saying
(02:24:04) mudrd8mz@jabber.cz/glum: I know, I just recapitulate :-)
(02:24:27) mudrd8mz@jabber.cz/glum: now I see what you meant by file_prepare_editor() does not work with $options['context']
(02:24:29) mudrd8mz@jabber.cz/glum: grrrrrrr
(02:25:02) Eloy: crazy unconnected world, lol.
(02:25:20) ***mudrd8mz@jabber.cz/glum must go out into the (rain) with the dog :-s
(02:25:24) mudrd8mz@jabber.cz/glum: ttyl
(02:25:29) Eloy: function MoodleQuickForm_editor

if (!$this->_options['context']) {
            $this->_options['context'] = get_context_instance(CONTEXT_SYSTEM);
        }

(02:25:32) Eloy: gogogo
(02:29:55) Eloy: so, summary, although not perfect.... perhaps file_prepare_editor() is the only place where we have both $options and $context to be compared for the 1, 2, 3 above. Won't cover all the editor uses but a lot of them. So my +1 to add that there.
(02:30:25) mudrd
(02:33:08) Eloy: yup
(02:37:04) Eloy: fyi, have updated tracker with our conclusions: http://tracker.moodle.org/browse/MDL-27500?focusedCommentId=113335&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-113335
(02:37:07) Eloy: 6 & 7
(02:37:17) mudrd8mz@jabber.cz/glum: (y)
(02:37:43) mudrd8mz@jabber.cz/glum: thanks!
(02:37:53) Eloy: I?m heading to bed. tomorrow I've easrly hours with little beast
(02:37:56) Eloy: ty !!!
(02:38:03) Eloy: *early hours
(02:38:13) Eloy: niao :-)
(02:38:16) mudrd8mz@jabber.cz/glum: niao
(02:44:57) Eloy: final zzzZZZzzz, see yo!
(02:45:04) mudrd8mz@jabber.cz/glum: & u