Tested by simply applying the patch and trying to edit an article
tinymce did NOT load and a 1 was visible top left

I've actually been working on some changes to the plugin. I wasn't going to make this particular kind of change but it's not a bad idea. I can incorporate it.

@brianteeman Sorry for that! didn’t catch it! Can you empty your browsers cache? Also what functionality you tried? Thanks

@okonomiyaki3000 Sounds excellent!

always test without any browser cache. Functionality tested as I described

On 21 January 2015 at 00:15, Dimitris Grammatiko notifications@github.com
wrote:

@brianteeman https://github.com/brianteeman Sorry for that! didn’t
catch it! Can you empty your browsers cache? Also what functionality you
tried? Thanks

—
Reply to this email directly or view it on GitHub
#5839 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@brianteeman Should be fine now!

Did a quick test and it worked but it is noticably slower than the existing method

See the video
https://www.dropbox.com/s/glr9d7lbgzx5gmw/editor.mp4?dl=0

@brianteeman I am experiencing the opposite here

@test
Works fine here and feels faster indeed.
Tiny: Any reason to take off JUri::root() . ?

Video doesnt lie
On 21 Jan 2015 09:09, "infograf768" notifications@github.com wrote:

@test https://github.com/test
Works fine here and feels faster indeed.
Tiny: Any reason to take off JPATH_ROOT . as it is kept elsewhere ?

—
Reply to this email directly or view it on GitHub
#5839 (comment).

@infograf768 You mean here: JHtml::_('script', $this->_basePath . '/tinymce.min.js'); ?

Yep

I think it will not work with the real absolute path, or am I wrong?

Was working before and it is used further in the same file.

Actually all the uses are for checks if files do exist e.g. :

            // Note this check for the template_list.js file will be removed in Joomla 4.0
            if (is_file(JPATH_ROOT . "/media/editors/tinymce/templates/template_list.js"))
            {
                // If using the legacy file we need to include and input the files the new way
                $str = file_get_contents(JPATH_ROOT . "/media/editors/tinymce/templates/template_list.js");

I see what you mean: JUri::root() . $this->_basePath . "/tinymce.min.js"
I will change it

Seems ok

(sorry for the confusion between JUri::root() and JPATH_ROOT in my comment above.

@brianteeman I don’t get that, even with repeated tests, different setups PHP etc. In my setup, worst case scenario, both ways are at par see video

But this is the way that Joomla sooner or later should follow for many reasons, here are some:
Scripts bottom
Require.js or any similar loader
Content Security Policy MDN
and a lot more cry for a definitive way to handle the scripts...

@infograf768 I think the last commit (include the JUri::root()) is actually against the common way of including scripts in Joomla.
This is the array of scripts as it is now (using JUri::root()):

And this is the same array but without the last commit:

I vote for consistency and thus make the path relative (as the rest of the scripts that Joomla uses).
But I don’t know if that would be a B/C break in any weird use case...

@dgt41 just use JUri::root(true) . '/' instead of JUri::root()

@Fedik was just testing it!

Sorry, slightly off topic... I've got some changes to the codemirror plugin that I'll be submitting soon and, as @dgt41 has suggested, I'm using addScriptDeclaration to put the editor instantiation code in the head of the document (in a ready function, specifically). I find that, compared with just instantiating the editor immediately, there can be a slight (almost 1 second) delay before rendering the editor. This seems to only happen in Debug mode probably because of the debug 'console' or whatever it is that appears at the bottom of the page. When not in Debug mode, there is no noticeable delay.

Any thoughts about this?

That is exactly what I found and reported in the video above. Didnt think to see if it was related to being in debug mode

My CodeMirror changes are available here #5863

@brianteeman would you be able to test if things get better if you disable debug mode please?

@dgt41 Whilst I agree that the tiny script should be added into the head i was under the impression that the intialization would be better under the text area to avoid dom ready (etc) checks needing to be added.

@wilsonge Both ways are working, but using always the API for all scripts can lead to an easy transition for scripts at the end of the page, or the usage of require.js and many other nice things. Yes there is a slight and noticeable delay when debug is on, but this is due to the fact that the DOM is inflated hugely with the infos and therefore the delay for parsing <textarea>. For example creating an article in the backend http://localhost/administrator/index.php?option=com_content&view=article&layout=edit without debug the served page weights: 92.89 KB and is inflated to 543.78 KB with debug on. I cannot count the DOM elements but you get the idea… Drawback? I wouldn’t say...

I agree with @dgt41

@wilsonge yeah it was the debug mode that caused the delay

When we want to load javascript code we have two options:

• Load it on from an external JS file
• Load it directly with addScriptDeclaration or whatever other method.

Personally when a script does not require to use any variable generated in PHP I would ALWAYS use external JS files. Why:

• They will load faster.
• They will be overridable in templates if we load them with JHtml::script.
• They can be minified
• The code can be highlighted and debugged properly in any editor with javascript language support

This is a great example:
https://github.com/joomla/joomla-cms/pull/5839/files#diff-7bc7fc0fdd702609aace0c8ed889aba8R29

That script doesn't need to be embed at all.

So I would move as much scripts as possible to: /media/plg_editor_none/

@phproberto Thanks Roberto for the input! I will follow your suggestion on the upcoming commit.
By the way, are you in favor or against the use of addScriptDeclaration instead of throwing scripts inside the body?

Cool. Thanks! :)

For the name I think I'd use the method name like:
/media/editors/none/js/on-init.js
/media/editors/none/js/on-get-insert-method.js (this can be moved also to an external file)

We should also add the media to the plugin manifest so it can be installed/uninstalled without leaving orphan files. Apparently all editor plugins are missing that

And if you want to fix more issues the codemirror plugin is loading the css directly from the plugin folder. Access to assets inside plugins folder is forbidden in robots.txt and then any asset loaded from there will throw an error in webmaster tools after the latest Webmaster Guidelines update:

Disallowing crawling of Javascript or CSS files in your site’s robots.txt directly harms how well our algorithms render and index your content and can result in suboptimal rankings.

See more in: http://googlewebmastercentral.blogspot.com.es/2014/10/updating-our-technical-webmaster.html

About JS loading my preference is: external js > addScriptDeclaration > embed script inside body

The reason is that when you are searching for JS you usually go to the head section of the page. And there are existing procesors that get head scripts and compress them. That can be done also within the body but it costs more to execute regular expressions there.

Is it really an issue for the codemirror css being in a folder that is
blocked by robots.txt. You are never going to have codemirror loaded in an
unauthenticated session so google will never see it loaded - will it?

On 3 February 2015 at 00:54, Roberto Segura notifications@github.com
wrote:

Cool. Thanks! :)

For the name I think I'd use the method name like:
/media/editors/none/js/on-init.js
/media/editors/none/js/on-get-insert-method.js (this can be moved also to
an external file)

We should also add the media to the plugin manifest
https://github.com/joomla/joomla-cms/blob/staging/plugins/editors/none/none.xml
so it can be installed/uninstalled without leaving orphan files. Apparently
all editor plugins are missing that [image: ]

And if you want to fix more issues the codemirror plugin is loading the
css directly from the plugin folder. Access to assets inside plugins folder
is forbidden in robots.txt and then any asset loaded from there will throw
an error in webmaster tools after the latest Webmaster Guidelines update:

Disallowing crawling of Javascript or CSS files in your site’s robots.txt
directly harms how well our algorithms render and index your content and
can result in suboptimal rankings.

See more in:
http://googlewebmastercentral.blogspot.com.es/2014/10/updating-our-technical-webmaster.html

About JS loading my preference is: external js > addScriptDeclaration > embed
script inside body

The reason is that when you are searching for JS you usually go to the
head section of the page. And there are existing procesors that get head
scripts and compress them. That can be done also within the body but it
costs more to execute regular expressions there.

—
Reply to this email directly or view it on GitHub
#5839 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

A case can be made for using CodeMirror to render code for display only. Such as this demo: http://codemirror.net/demo/runmode.html

You'd want the css in that case.

Ok. In that case @phproberto is correct

in most cases I agree with @phproberto
but I have some notes:
I am against make a file just for ONE method, if we start make such thing we end up with 100 JS files at one page,

yes, maybe one small file will load fast and it not a problem, but if you have a 30 files and a mobile connection with huge Ping, it will loaded not so fast ... and people will hate mobile joomla

page with one small "inline" method will load faster than same page with this method in js file,
same about CSS

all should be efficient,
in current case all JS can be merged into ONE file

PS. into one but separated by extension, eg /none/none-scripts.js /tinymce/tinymce-scripts.js

@Fedik you are right about the impact of increasing the http requests. Personally I don't mind this approach as I always use in my templates an option for scripts concatenation and compression. What really bothers me are script inserted on the page's body!
By the way for this particular pr I think is ok for few reasons:
1.hardly anyone will use mobile to actually commit something
2. Most of the times (if sever has properly defined expires) browser cache will kick in!

Sorry @Fedik is right. I didn't notice that this is just loading 2 functions. And yes both can be inside the same JS file

About names I think none-scripts.js is not very descriptive. I'd rather use none-functions.js or just do what @dgt41 did originally and call it directly none.js

@Fedik @phproberto Thanks for the input, should be fine now

Latest commit is just two vars fro tinyMCE that: explicitly declare the path to tinyMCE script and that the script got a suffix .min.
Why? This PR broke my own (and possible every other) plugin for script concatenation and compression.

Already implemented for tinyMCE @ #7170, for none will do another PR and another for the manifests.

So closing it here