Skip to content

Conversation

@OctavianC
Copy link
Contributor

Summary of Changes

Because PluginHelper::$plugins is defined as static, getPlugin() returns a reference to the original $plugin object. When _loadEditor() is invoked it will load the params into the Registry and overwrite the original $plugin->params with the Registry object. Subsequent calls to _loadEditor() will try to load the modified $plugin->params and will lead to unexpected results:

First call:

object(Joomla\Registry\Registry)#417 (3) {
  ["data":protected]=>
  object(stdClass)#422 (11) {
    ["lineNumbers"]=>
    string(1) "1"
    ["lineWrapping"]=>
    string(1) "1"
    ["matchTags"]=>
    string(1) "1"
    ["matchBrackets"]=>
    string(1) "1"
    ["marker-gutter"]=>
    string(1) "1"
    ["autoCloseTags"]=>
    string(1) "1"
    ["autoCloseBrackets"]=>
    string(1) "1"
    ["autoFocus"]=>
    string(1) "1"
    ["theme"]=>
    string(7) "default"
    ["tabmode"]=>
    string(6) "indent"
    ["syntax"]=>
    string(3) "php"
  }
  ["initialized":protected]=>
  bool(true)
  ["separator"]=>
  string(1) "."
}

Second call:

object(Joomla\Registry\Registry)#423 (3) {
  ["data":protected]=>
  object(stdClass)#425 (4) {
    ["data"]=>
    object(stdClass)#426 (11) {
      ["lineNumbers"]=>
      string(1) "1"
      ["lineWrapping"]=>
      string(1) "1"
      ["matchTags"]=>
      string(1) "1"
      ["matchBrackets"]=>
      string(1) "1"
      ["marker-gutter"]=>
      string(1) "1"
      ["autoCloseTags"]=>
      string(1) "1"
      ["autoCloseBrackets"]=>
      string(1) "1"
      ["autoFocus"]=>
      string(1) "1"
      ["theme"]=>
      string(7) "default"
      ["tabmode"]=>
      string(6) "indent"
      ["syntax"]=>
      string(3) "php"
    }
    ["initialized"]=>
    bool(true)
    ["separator"]=>
    string(1) "."
    ["syntax"]=>
    string(10) "javascript"
  }
  ["initialized":protected]=>
  bool(true)
  ["separator"]=>
  string(1) "."
}

As you can see, data is now inside another data object.

Testing Instructions

Let's say you want to display two CodeMirror editors - one for PHP, the other one for Javascript. For the sake of it, I'll make the code simple:

$instance = new JEditor('codemirror');
echo $instance->display('first_editor', '<?php echo "test"; ?>', '100%', 300, 75, 20, $buttons = false, $id = null, $asset = null, $author = null, array('syntax' => 'php'));

$instance2 = new JEditor('codemirror');
echo $instance2->display('second_editor', 'var test;', '100%', 300, 75, 20, $buttons = false, $id = null, $asset = null, $author = null, array('syntax' => 'javascript'));

Expected result

Two editors with different syntax highlighting - no other differences.

Actual result

Two editors but the second editor grabbing incorrect plugin parameters (no line numbers, no gutters etc).

Documentation Changes Required

Hopefully none?

@OctavianC OctavianC changed the title Needs to be a clone or params get messed up Multiple editors don't inherit $params correctly Jan 26, 2018
@OctavianC
Copy link
Contributor Author

Sorry, didn't notice the title was taken from the commit :)

@Quy
Copy link
Contributor

Quy commented Feb 3, 2018

I have tested this item ✅ successfully on 5c27563


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19454.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 5c27563


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19454.

@Quy
Copy link
Contributor

Quy commented Feb 3, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19454.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 3, 2019
@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@alikon
Copy link
Contributor

alikon commented May 2, 2019

@HLeithner please a final response

1 year old rtc pr

@HLeithner
Copy link
Member

As far as I know is our plugin system a singleton so the editor plugins have to deal with this correctly. This seams like a wrong workaround.

ymmv

@OctavianC
Copy link
Contributor Author

It's been more than a year - this has been indirectly solved through #20583 (Registry update). Original commit here:

joomla-framework/registry@3813e93

So it can be closed.

@OctavianC OctavianC closed this May 13, 2019
@joomla-cms-bot joomla-cms-bot added PR-staging and removed RTC This Pull Request is Ready To Commit labels May 13, 2019
@OctavianC OctavianC deleted the patch-9 branch November 23, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants