I was gonna say this is wrong because it invalidates all of the app config
values set up in the constructor. But honestly, I'm at the screw it
stage. Just merge this and keep accepting that the CMS config references
are fatally broken.

On Thursday, October 6, 2016, zero-24 notifications@github.com wrote:

Summary of Changes

Init the config value to load the correct editor.
Testing Instructions

• Install 3.6.3-rc2
• set up your default editor (global config) to tinyMCE
• go to your user config
• set the editor to "codemirror"
• try to create or edit an article.
• you notice that the tinymce is loaded
• apply this patch
• the codemirror is loaded (as per user setting)

Documentation Changes Required

None
Other things

@mbabker https://github.com/mbabker this reversts part of: #11838
#11838 can you take a look

here maybe there is a better fix?

You can view, comment on, or merge this pull request online at:

#12330
Commit Summary

• restore the editor handling

File Changes

• M libraries/cms/application/cms.php https://github.com/joomla/joomla-cms/pull/12330/files#diff-0 (3)

Patch Links:

• https://github.com/joomla/joomla-cms/pull/12330.patch
• https://github.com/joomla/joomla-cms/pull/12330.diff

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12330, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoUfB8IQpFUjl6UkiryAyf57RNBdpks5qxWzrgaJpZM4KQdII
.

And just for reference the commented change in JApplicationWeb::redirect()
is in part caused by this behavior of overloading the app config registry
with the JConfig registry.

Accept this broken behavior. At some point JFactory::getConfig() needs to
be deprecated and proxy to the app class.

On Thursday, October 6, 2016, zero-24 notifications@github.com wrote:

Summary of Changes

Init the config value to load the correct editor.
Testing Instructions

• Install 3.6.3-rc2
• set up your default editor (global config) to tinyMCE
• go to your user config
• set the editor to "codemirror"
• try to create or edit an article.
• you notice that the tinymce is loaded
• apply this patch
• the codemirror is loaded (as per user setting)

Documentation Changes Required

None
Other things

@mbabker https://github.com/mbabker this reversts part of: #11838
#11838 can you take a look

here maybe there is a better fix?

You can view, comment on, or merge this pull request online at:

#12330
Commit Summary

• restore the editor handling

File Changes

• M libraries/cms/application/cms.php https://github.com/joomla/joomla-cms/pull/12330/files#diff-0 (3)

Patch Links:

• https://github.com/joomla/joomla-cms/pull/12330.patch
• https://github.com/joomla/joomla-cms/pull/12330.diff

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12330, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoUfB8IQpFUjl6UkiryAyf57RNBdpks5qxWzrgaJpZM4KQdII
.

Yes i have seen this too but i have no idea how to fix it right?

If i got it right the cMS does not inject a config before this line?
As the getinstance method does not set the constructor settings?
https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/application/cms.php#L396

Maybe i'm also just on the complete wrong way?

Hmm i dod not comment code? What commented code do you mean?

It's been there for years, not related to this at all.

On Thursday, October 6, 2016, zero-24 notifications@github.com wrote:

Hmm i dod not comment code? What commented code do you mean?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12330 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoWYr00lcmq5hkwgMD6RwKDC2j0oNks5qxXa0gaJpZM4KQdII
.

Sorry our comments crossed :) Which question you answer with the last comment? The first or the seccond or both?

Part of the problem is until the line you added back, there are essentially
two different configuration objects in play. The registry from JFactory
and the one created by the app. The line you've added back essentially
turns the app's registry into a reference to the registry in JFactory so
that they both use the same data object. This invalidates all config
values set in the app constructor (session related stuff, and some things
the CMS never really used, but that's really beyond the point here).

So there are a few possible fixes. JFactory::getApplication() or
JApplicationCms::getInstance() could possibly inject in
JFactory::getConfig() which would force the reference behavior to be set
correctly. Or the JApplicationCms constructor get the
JFactory::getConfig() registry to use that as the class' config value if
nothing else is injected in (but only if nothing is injected otherwise we
break the parameter injection in the constructor).

I doubt many will want to test the fixes, generally any code structure
change I suggest or implement gets reverted for some edge case scenario
that stops working or ignored because it's too technical for most. So
accept reverting to a known buggy state and be done with it, which is what
this PR is doing.

On Thursday, October 6, 2016, zero-24 notifications@github.com wrote:

Yes i have seen this too but i have no idea how to fix it right?

If i got it right the cMS does not inject a config before this line?
As the getinstance method does not set the constructor settings?
https://github.com/joomla/joomla-cms/blob/staging/
libraries/cms/application/cms.php#L396

Maybe i'm also just on the complete wrong way?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12330 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoUFhrT6MfiUPQWm2cH9_CTQ7lkKpks5qxXY_gaJpZM4KQdII
.

The "it's been there for years" comment related to the redirect method
change. Joys of answering by email on phone, don't see the notification
for new messages.

On Thursday, October 6, 2016, zero-24 notifications@github.com wrote:

Sorry our comments crossed :) Which question you answer with the last
comment? The first or the seccond or both?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12330 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoSPSx5be0duSAjIJq8UQyK0-qV0Vks5qxXefgaJpZM4KQdII
.

I'm very sorry i don't want revert your code for a non reason, it was just the try an error way that get me to your line / change and i agree with you that this is not the correct fix. I need to go to bed now but thank you for your technical input. I can try it later and send a better PR (if that how i understand your comments correct works) ?

This is almost the same thing that I broke all the user plugins with last time #3758 :P Which we ended up reverting for lack of any better ideas.... Joomla 4 is the solution sadly