r98892 MediaWiki - Code Review archive

Should probably do one of the following:

1. throw an exception if given bogus non-string input, so we don't explode here or several levels deeper in the code
2. force input to a string before we process it, so we don't explode here or several levels deeper in the code

This doesn't seem to relate clearly to r98578 either; that rev only changes <textarea> output, and doesn't affect anything that's been passing different data into EditPage::showTextbox...?

This fixes the issue that r98578 caused in core. You know, showTextbox uses textarea too. I know it's not pretty but I couldn't come up with better one on this short time.

I'm just not sure why anything would try to pass null instead of text into an EditPage instance deliberately. I assume it was just left returning null by mistake?

Problem seems to come about by directly manipulating an EditPage instance's member variables in a hook, so there's no interface boundary for EditPage to validate the input.

TranslateEditAddons::editBoxes() is setting $object->textbox1 directly... looks like normally it gets set inside (from Article::getContent() in EditPage::initialiseForm() or via EditPage::safeUnicodeInput from form input). It can in some places be set via an outparam on EditFormPreloadText, which should probably validate.

The hook appears to be getting called via 'EditPage::showEditForm:initial'.... editBoxes() seems to change the initial text as a side-effect, and also return some HTML chunks to display somewhere?

Would the initial text be better set via the EditFormInitialText or EditFormPreloadText hooks which appear to exist for that sort of purpose? We could then do some validity checks around them, or add a public method to set the text which would validate which the hook can use.

I have to check those hooks. I do have to mention that it has been very tedious process trying to override the edit field value without causing problems elsewhere, like saving wrong content, overwriting user changes on preview and such and I would appreciate if an easy way to set the default value existed. Maybe those hooks have been added after I wrote this code, but like I said I need to check if they work and are in 1.17.

*nod* -- let's swap this from a fixme to a todo; that may include improving the EditPage hook interfaces for future. :D

With the other fixes the current code here looks fine for now.