Bug 9172 - Move TinyMCE library outside of language-specific directory
Summary: Move TinyMCE library outside of language-specific directory
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 9926
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-29 20:32 UTC by Owen Leonard
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2012-11-29 20:32:23 UTC
The TinyMCE library should be moved to intranet-tmpl/lib so that it doesn't have to be duplicated for each translation and so that the translation script won't have to parse it each time.

Rather than move the existing library I propose that the old copy be removed an an upgraded version be added to intranet-tmpl/lib/tiny_mce. I also propose that we do not distribute the docs and examples directories, reducing further TinyMCE's overall footprint.
Comment 1 Owen Leonard 2012-11-29 20:34:03 UTC
Oh, another proposed change: To create a configuration include in which to specify translatable strings just as we do with the DatePicker and calendar.inc.
Comment 2 Owen Leonard 2012-11-30 20:21:59 UTC
The patch for this bug is too large to upload (2.6MB), but there is a branch for it on my Gitorious repo (http://gitorious.org/koha-dev/koha-dev) called ps-bug-9172-tinymce-lib-path-2012-11-30
Comment 3 Owen Leonard 2013-02-06 14:30:31 UTC
The branch has been rebased against current master to resolve conflicts. Here is the commit message since I can't attach the patch:

    Bug 9172 - Move TinyMCE library outside of language-specific directory
    
    This patch removes the TinyMCE library from its language-specific
    location and puts it in intranet-tmpl/lib. The most recent version
    of TinyMCE is used, and the files added have been limited to
    those required for proper functionality of the editor.
    
    Files omitted: documentation, examples, unused plugins.
    
    To test, visit each page which uses the TinyMCE editor: News
    editing (tools/koha-news.pl), library editing (admin/branches.pl),
    and online help editing (help/edithelp.pl). The editor should
    look and function as before.
Comment 4 Bernardo Gonzalez Kriegel 2013-02-26 23:26:37 UTC
[SIGNED-OFF]Bug 9172 - Move TinyMCE library outside of language-specific directory

Comment: Work as described. Koha complains about forbidden pattern (====),
but is a false positive, just comments in function definition. Could be removed,
ej
#===================================
sub get_params {
#===================================

Test
1) git clone git://gitorious.org/koha-dev/koha-dev.git kohaclone
2) git checkout -b bug_9172 origin/ps-bug-9172-tinymce-lib-path-2012-11-30
3) Editor works as usual in News, Help and Branch editing

How to merge.. is not my problem :)
Comment 5 Mason James 2013-03-26 06:02:55 UTC
(In reply to comment #4)
> [SIGNED-OFF]Bug 9172 - Move TinyMCE library outside of language-specific
> directory
> 
> Comment: Work as described. Koha complains about forbidden pattern (====),
> but is a false positive, just comments in function definition. Could be
> removed,
> ej
> #===================================
> sub get_params {
> #===================================

 
QA comments...
the patch looks good, and works perfectly

just one change before passing QA, can we please replace or delete the above mentioned string, used for detecting merge conflicts in files

ps: i think this patch is a really a bugfix, rather than enhancement :)
Comment 6 Mason James 2013-03-26 09:20:02 UTC
> QA comments...
> the patch looks good, and works perfectly
> 
> just one change before passing QA, can we please replace or delete the above
> mentioned string, used for detecting merge conflicts in files
> 
> ps: i think this patch is a really a bugfix, rather than enhancement :)


also, this patch depends on 9926
Comment 7 Owen Leonard 2013-04-02 13:10:02 UTC
(In reply to comment #5)

> > #===================================
> > sub get_params {
> > #===================================

Since these lines are in a file which is part of the TinyMCE distribution I hesitate to modify them. I notice too that we have a couple of examples of that construction in existing Koha scripts:

kohaclone/authorities/authorities.pl:
  535  # ======================== 
  536  #          MAIN 
  537: #=========================
  538  my $input = new CGI;
  539  my $z3950 = $input->param('z3950');

kohaclone/cataloguing/addbiblio.pl:
  714  # ========================
  715  #          MAIN
  716: #=========================
  717  my $input = new CGI;
  718  my $error = $input->param('error');
Comment 8 Bernardo Gonzalez Kriegel 2013-04-02 15:59:57 UTC
(In reply to comment #7)
> Since these lines are in a file which is part of the TinyMCE distribution I
> hesitate to modify them. I notice too that we have a couple of examples of
> that construction in existing Koha scripts:

Sorry for my comment, as Owen points there are other files with "===" marks.
I think that this may pass, but of course is up to the QA people and RM.
Comment 9 Mason James 2013-04-02 21:16:01 UTC
(In reply to comment #7)
> (In reply to comment #5)
> 
> > > #===================================
> > > sub get_params {
> > > #===================================
> 
> Since these lines are in a file which is part of the TinyMCE distribution I
> hesitate to modify them.

yes, i agree :)

so, now passing QA on this...
Comment 10 Jared Camins-Esakov 2013-04-03 10:33:12 UTC
I will need the QAed branch for merging. Mason, could you make it available somewhere?
Comment 11 Mason James 2013-04-03 19:31:52 UTC
(In reply to comment #10)
> I will need the QAed branch for merging. Mason, could you make it available
> somewhere?


hmmm, using Owens repo/branch should still apply fine to master
(from memory, i had one tiny merge conflict)

http://gitorious.org/koha-dev/koha-dev 
branch is 'ps-bug-9172-tinymce-lib-path-2012-11-30'

will that be OK?
Comment 12 Mason James 2013-04-04 02:32:16 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > I will need the QAed branch for merging. Mason, could you make it available
> > somewhere?

i have signed off on this patch, 

its available for merging here...
 https://github.com/KohaAloha/Koha-Dev/tree/bug_9172
Comment 13 Jared Camins-Esakov 2013-04-15 13:32:37 UTC
This patch has been pushed to master and 3.12.x.
Comment 14 Robin Sheat 2013-04-15 22:55:35 UTC
fyi this patch seems to break package builds.