Page MenuHomePhabricator

Internal bug in FCKEditor extension
Closed, InvalidPublic

Description

Author: aperry

Description:
I've installed Mediawiki 1.15.1 on Wampp 1.7.2 (Apache 2.2, PHP 5.3.0, MySQL 5.1).
Then I've installed mediawiki FCKEditor extension.

When I want to edit a page, I get internal error :

"Detected bug in an extension! Hook FCKEditor_MediaWiki::onCustomEditor failed to return a value; should return true to continue hook processing or false to abort".

Crawling on the Net, I noticed others guys with the same bug, but no true way to fix the problem.

So played again the installation, but with a Xampp version embedding PHP 5.2.2.
This time, everythings look fine : I can use FCKEditor during editing a page.

My conclusion is that there is incompatibility with the mediwiki FCKEditor version and PHP 5.3.

Do you think I'm right ? because it's very strange a so big bug is not yet fixed.

Regards


Version: unspecified
Severity: blocker

Details

Reference
bz21907

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:56 PM
bzimport set Reference to bz21907.
bzimport added a subscriber: Unknown Object (MLST).

pdhanda wrote:

Hi,

I followed these instructions:
http://mediawiki.fckeditor.net/index.php/FCKeditor_integration_guide

with PHP 5.3.0., MySQL 5.0.88, apache2 and FCKeditor 2.6.5.

I had no problems in Firefox or Safari. However I tested this with 1.16alpha. Let me try with the version of Mediawiki you are using and get back to you.

-p

Weird. FCKEditor just seems to have gotten some 5.3.x updates in r50690, but they seem unrelated.

In general, this bug means that something is using a hook (in this case, the method FCKEditor_MediaWiki::onCustomEditor is using the CustomEditor hook) but not returning a boolean value to continue/halt processing. Looking at the trunk version of things, it seems like we're returning false. Odd.

Bug needs verification on trunk, I haven't looked at the out-of-repo site.

pdhanda wrote:

Yeah, looks like a problem in Mediawiki 1.15.1 that was fixed in trunk. I'm new here, So I'm not entirely sure how we handle this. I'll see if I can isolate the fix and I'll chat with Tim about how to proceed.

-p

(In reply to comment #3)

Yeah, looks like a problem in Mediawiki 1.15.1 that was fixed in trunk. I'm new
here, So I'm not entirely sure how we handle this. I'll see if I can isolate
the fix and I'll chat with Tim about how to proceed.

-p

Maybe a backport to the REL1_15 branch (since it's a blocker fix), then close this as FIXED.

aperry wrote:

Hi, thanks for all this tracking.
What I understand is that the problem comes from Mediawiki version management.
Is the bug near to be closed and FIXED ?
Thanks a lot for infos.

(In reply to comment #3)

Yeah, looks like a problem in Mediawiki 1.15.1 that was fixed in trunk. I'm new
here, So I'm not entirely sure how we handle this. I'll see if I can isolate
the fix and I'll chat with Tim about how to proceed.

-p

Not so sure it's purely "fixed in trunk but broken in release" either. Checked out REL1_15 earlier today and the code in onCustomEditor seems to still return false. Will play more with it later.

(In reply to comment #5)

Hi, thanks for all this tracking.
What I understand is that the problem comes from Mediawiki version management.
Is the bug near to be closed and FIXED ?
Thanks a lot for infos.

Not really sure yet. Need to verify that this truly is indeed fixed (or indeed still broken, in which case we can fix it).

d.r.newman wrote:

I can confirm the same error with Mediawiki 1.15.1 and SMWHalo's version of FCKEditor.

I also get the same error with SMWHalo when editing user preferences. That is also a case
where a hook seems to return null and trigger the error reported above.

d.r.newman wrote:

I seem to have found a way of fixing the bug on my setup. It is probably not the complete solution,
but it might help a better programmer find a better solution.

In FCKeditor.body.php the custom editor function is defined as:

public function onCustomEditor(&$article, &$user)

But in Wiki.php, there is a line that says:

if( wfRunHooks( 'CustomEditor', array( $article, $user ) ) ) {

When I changed one of them to make either both calls pass by reference, or both
calls pass by value, the error went away, and the FCK editor popped up.

I wonder if this mismatch has been caught by one of the backward incompatibilities
of PHP 5.3 (http://uk3.php.net/manual/en/migration53.incompatible.php)?

aperry wrote:

Hi support guys,

Well, nice idea from David ; but is this the fix to the problem ?

aperry wrote:

Hi,

I did what David proposed about calls pass by reference or by value.
But, unfortunatly, it does not work, with Mediawiki 1.15.1 and PHP 5.3

Ans the FCKEditor web site doesn't propose any new version.

d.r.newman wrote:

Yayayoute.

It solved my problems with exactly those versions. So there must be some other factor that makes a difference.

What exactly did you do? Did you change

public function onCustomEditor(&$article, &$user)

to

public function onCustomEditor($article, $user)

or did you change

if( wfRunHooks( 'CustomEditor', array( $article, $user ) ) ) {

to

if( wfRunHooks( 'CustomEditor', array( &$article, &$user ) ) ) {

?

Ok, there seem to be two bugs being discussed here at once. Let me try to make sense of it all.

(In reply to comment #0)

"Detected bug in an extension! Hook FCKEditor_MediaWiki::onCustomEditor failed
to return a value; should return true to continue hook processing or false to
abort".

As I said in comment #2, this bug happens when a hook function (in this case: FCKEditor_MediaWiki::onCustomEditor using the CustomEditor hook) fails to return a boolean to continue or halt further processing. I have checked both trunk and the REL1_15 branch (as of r61540) and the onCustomEditor method seems to be returning false, as it should. Can you please check your copy for this? If it's not returning false, try returning false and see if that fixes it.

(In reply to comment #8)

In FCKeditor.body.php the custom editor function is defined as:

public function onCustomEditor(&$article, &$user)

But in Wiki.php, there is a line that says:

if( wfRunHooks( 'CustomEditor', array( $article, $user ) ) ) {

The hook is correct and the extension is wrong. Neither $article or $user should be passed by reference. This was fixed in trunk in r50690. This portion of the bug is FIXED.

aperry wrote:

Hi,
Well, I made a totally new implementation of Xampp 1.7.2 (Apache 2.2, PHP 5.3.0, MySQL 5.1).

I installed FCKEditor from mediawiki_fckeditor_ext_N.zip which seems to be a specific version of FCKEditor for MediaWiki.

Without any change in configuration files, if I want to modify a Wiki page, I get the error :

Internal Error

Set $wgShowExceptionDetails = true; at the bottom of LocalSettings.php to show detailed debugging information.

But, if I follow David's proposal, I now get both native TextWiki editor or Rich Editor working fine and I can swap from one to the other.
I should say that both David's proposal (by addressing by value in hook and extension or addressing by ref in hook and extension) give a good result.

So I decide to follow Chad's : no addressing by reference in the extension.

I add, that, natively, public function onCustomEditor($article, $user) return false;

Well, thanks a lot, guys, you are full of fight.

Regards.

Nota : very odd FCKEditor's community has not fixed the trouble.

jcuzella wrote:

I too am seeing problems due to PHP 5.3.x and current versions of both MediaWiki and FCKEditor. Thanks to XDebug, I'm actually getting PHP errors to try and track this down. Other good news: It seems that I've fixed all but one last issue relating to pass by reference use within FCKEditor's source. Right now I'm a bit stumped because I'm really not too familiar with FCKEditor's code, or how these functions are supposed to work.

Here's what I've done so far:

  1. Installed FCKEditor (r5859)
  2. Fixed some strict PHP errors by changing the function definitions in a couple child classes to match that of their parents. (Also seems someone else has run into this in bug 23405)
  3. Created a patch on r5859 with these fixes
  4. Run into one last error:

Strict standards: Only variables should be passed by reference in C:\xampp\htdocs\mediawiki\extensions\FCKeditor\FCKeditorParserOptions.body.php on line 9

Please see FCKEditor's talk page on MediaWiki.org for full documentation of what I've done to debug this so far:
http://www.mediawiki.org/wiki/Extension_talk:FCKeditor_%28Official%29#1.16_and_latest_revision_not_working

I will also attach my patch here.

jcuzella wrote:

Partial patch made based off of SVN r5859

For more details, see:

http://www.mediawiki.org/wiki/Extension_talk:FCKeditor_%28Official%29#Semi-working_Patch

Attached:

jcuzella wrote:

mylyn/context/zip

Attached:

jcuzella wrote:

PLEASE NOTE: The revision I reference (r5859) is *not* the one that this bugtracker auto-linkified to the main mediawiki SVN. Revisions referenced earlier in this thread were much larger than I was expecting, due to being in the main MW repo ;-)

It is the remote revision of the svn:external repo for FCKeditor: http://svn.fckeditor.net/MediaWiki/trunk/

jcuzella wrote:

Looked into this further today, and my current issue seems to be with the constructor for the FCKeditorSkin class.

It's defined in FCKeditorSkin.body.php:

function __construct( &$skin ) {

		$this->skin = $skin;

}

I changed the function definition to the following, and it seems to work for me now:

function __construct( $skin ) {


I just reviewed the new instructions at http://www.mediawiki.org/wiki/Extension:FCKeditor_%28Official%29
There were some changes made since I had originally followed them at the beginning of last month. Most notably, the note regarding the MW extension being forked from the upstream version at: http://svn.fckeditor.net/MediaWiki/trunk/

This bug seems very old, but is still appearing in current source from upstream... it's strange they still haven't fixed this one, because it's a showstopper (at least for the MW extension).

I think it would be ideal if we could port these changes back upstream in order to avoid confusion arising from installing this extension from different sources, and so upstream can benefit too.

jcuzella wrote:

Full patch based off of upstream SVN r5861

Created new patch based off of SVN r5861. This one includes my last fix to the constructor function parameter.

Attached:

jcuzella wrote:

I reported this upstream at: http://dev.ckeditor.com/ticket/5602

I also opened a ticket regarding the forked project, and a plan to re-merge it: http://dev.ckeditor.com/ticket/6273

Is there any reason to keep this fork if the bugs fixed in it are fixed upstream?

jcuzella wrote:

*** Bug 21332 has been marked as a duplicate of this bug. ***

*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*

Per the upsteam bug tracker "FCKeditor and MediaWiki are no longer supported. Closing the ticked as invalid."