Bug 25485 - TinyMCE broken in Debian package installs
Summary: TinyMCE broken in Debian package installs
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 25068
Blocks: 25529
  Show dependency treegraph
 
Reported: 2020-05-13 14:26 UTC by Kyle M Hall
Modified: 2020-05-22 14:51 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06


Attachments
Bug 25485: TinyMCE broken in Debian package installs (1.80 KB, patch)
2020-05-13 14:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25485: Remove all mention of tiny_mce in debian/rules (879 bytes, patch)
2020-05-18 03:29 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: Remove tiny_mce symlink during pre install of deb package (1.06 KB, patch)
2020-05-18 05:42 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: TinyMCE broken in Debian package installs (1.84 KB, patch)
2020-05-18 06:00 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: TinyMCE broken in Debian package installs (1.84 KB, patch)
2020-05-18 06:03 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: Remove all mention of tiny_mce in debian/rules (933 bytes, patch)
2020-05-18 06:03 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: Remove tiny_mce symlink during pre install of deb package (1.12 KB, patch)
2020-05-18 06:03 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: [Do not push] remove tiny_mce from debian/control for testing (700 bytes, patch)
2020-05-18 06:04 UTC, David Cook
Details | Diff | Splinter Review
Bug 25485: Remove all mention of tiny_mce in debian/rules (989 bytes, patch)
2020-05-18 21:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 25485: Remove tiny_mce symlink during pre install of deb package (1.17 KB, patch)
2020-05-18 21:13 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 25485: [Do not push] remove tiny_mce from debian/control for testing (702 bytes, patch)
2020-05-18 21:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 25485: TinyMCE broken in Debian package installs (1.89 KB, patch)
2020-05-19 22:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25485: Remove all mention of tiny_mce in debian/rules (1.02 KB, patch)
2020-05-19 22:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25485: Remove tiny_mce symlink during pre install of deb package (1.23 KB, patch)
2020-05-19 22:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2020-05-13 14:26:13 UTC
When installing the Debian package of Koha, we install the Debian tinymce package and symlink over our own tinymce libs that are in the Koha codebase. This version of tinymce does not have the same files and because of this, tinymce does not get loaded.

We should remove the use of the tinymce Debian package, and just use our own version of TinyMCE.
Comment 1 Kyle M Hall 2020-05-13 14:28:27 UTC
Created attachment 104849 [details] [review]
Bug 25485: TinyMCE broken in Debian package installs

When installing the Debian package of Koha, we install the Debian tinymce package and symlink over our own tinymce libs that are in the Koha codebase. This version of tinymce does not have the same files and because of this, tinymce does not get loaded.

We should remove the use of the tinymce Debian package, and just use our own version of TinyMCE.

Test Plan:
1) Install Koha 19.11 from package
2) Browse to the news editor
4) Note the WYSIWYG editor is missing
5) Build a new Koha package with this patch applied, install it
6) Reload the news editor
7) Note the WYSIWYG editor is back!
Comment 2 Katrin Fischer 2020-05-14 15:38:07 UTC
Hi Kyle, I am seeing a 'partial' breakage in 18.11, could this be related? TinyMCE loads, but when I click on 'link' for example, the pop-up is not formatted nicely and the dev tools report at least 2 files missing, one being:
/cgi-bin/koha/tools/themes/advanced/skins/default/dialog.css
Comment 3 Victor Grousset/tuxayo 2020-05-14 17:14:06 UTC
If you don't have the same Koha and/or Debian version we can expect variations. Like only the CSS file missing while the rest is not broken.
Comment 4 Tomás Cohen Arazi 2020-05-14 22:50:07 UTC
This is amazing.
Comment 5 David Cook 2020-05-14 23:59:31 UTC
(In reply to Tomás Cohen Arazi from comment #4)
> This is amazing.

I know, right?

I've often wondered why we used Debian's tinymce package. I think we used to do the same thing with the yui Javascript. 

And we're requiring Debian's libjs-jquery package too, although I don't think that actually gets used anywhere?
Comment 6 David Cook 2020-05-15 02:31:38 UTC
(In reply to Kyle M Hall from comment #1)
> Test Plan:
> 1) Install Koha 19.11 from package
> 2) Browse to the news editor
> 4) Note the WYSIWYG editor is missing
> 5) Build a new Koha package with this patch applied, install it
> 6) Reload the news editor
> 7) Note the WYSIWYG editor is back!

I don't think that this test plan is 100% sufficient, but it is a hard one to explain well...
Comment 7 David Cook 2020-05-15 02:32:19 UTC
Comment on attachment 104849 [details] [review]
Bug 25485: TinyMCE broken in Debian package installs

Review of attachment 104849 [details] [review]:
-----------------------------------------------------------------

::: debian/rules
@@ +23,3 @@
>  	rm $(TMP)/var/log/koha/README
>  	rm -r \
>  		$(TMP)/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce

At a glance, it looks like this should cause a problem? But maybe I'm misreading the rules file.
Comment 8 David Cook 2020-05-15 02:58:30 UTC
(In reply to David Cook from comment #7)
> Comment on attachment 104849 [details] [review] [review]
> Bug 25485: TinyMCE broken in Debian package installs
> 
> Review of attachment 104849 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: debian/rules
> @@ +23,3 @@
> >  	rm $(TMP)/var/log/koha/README
> >  	rm -r \
> >  		$(TMP)/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce
> 
> At a glance, it looks like this should cause a problem? But maybe I'm
> misreading the rules file.

So I removed that rule in my test, but that also meant the existing symlink didn't get removed... so I had to unlink the symlink and then reinstall the koha-common package...
Comment 9 David Cook 2020-05-15 03:02:30 UTC
(In reply to Kyle M Hall from comment #1)
> Test Plan:
> 1) Install Koha 19.11 from package
> 2) Browse to the news editor
> 4) Note the WYSIWYG editor is missing
> 5) Build a new Koha package with this patch applied, install it
> 6) Reload the news editor
> 7) Note the WYSIWYG editor is back!

As I noted above, I found this wasn't a sufficient test plan.

I did something like this:

0) Applied Kyle's patch + manually updated debian/control and removed a questionable line in debian/rules
1) Built the koha-common package
2) Unlinked /usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce in my target environment
3) Installed (ie upgraded) Koha package
4) Noted the WYSIWYG editor is back!

So... not sure this one is quite there yet.
Comment 10 David Cook 2020-05-15 03:06:43 UTC
Actually, I'll do more testing without my changes... going back to "Needs Signoff"
Comment 11 Victor Grousset/tuxayo 2020-05-15 03:43:56 UTC
(In reply to David Cook from comment #5)
> And we're requiring Debian's libjs-jquery package too, although I don't
> think that actually gets used anywhere?

git grep "/usr/share/" | grep -vE "/usr/share/koha|/usr/share/perl5|/usr/share/fonts|/usr/share/debconf|/usr/share/doc/koha"

IIUC, nothing to worry about :D


Makefile.PL:    push @version, ('/usr/share/perl/5.10') if !$version[0];
debian/copyright: /usr/share/common-licenses/GPL-3 .
debian/koha.config:if [ -f /usr/share/dbconfig-common/dpkg/config.mysql ]; then
debian/koha.config:	. /usr/share/dbconfig-common/dpkg/config.mysql 
debian/koha.postinst:. /usr/share/dbconfig-common/dpkg/postinst.mysql 
debian/koha.postrm:if [ -f /usr/share/dbconfig-common/dpkg/postrm.mysql ]; then
debian/koha.postrm:    . /usr/share/dbconfig-common/dpkg/postrm.mysql
debian/koha.prerm:. /usr/share/dbconfig-common/dpkg/prerm.mysql 
debian/rules:	ln -s /usr/share/tinymce/www \
debian/rules:	    /usr/share/xml/docbook/stylesheet/docbook-xsl/manpages/docbook.xsl \
Comment 12 David Cook 2020-05-15 07:21:03 UTC
I think this one needs more testing, but on my initial tests it doesn't work when upgrading Koha. I imagine it would work for installing Koha initially though.
Comment 13 David Cook 2020-05-15 07:28:26 UTC
Ok when I removed everything tiny_mce related from debian/rules, it installed the bundled tinymce overtop of /usr/share/tinymce/www through the existing symlink for my already existing Koha installation. Yikes.

I should've tested with Kyle's original patch, but it's overtime on Friday night, so will have to leave that to someone else, or I can revisit on Monday...
Comment 14 Katrin Fischer 2020-05-15 14:22:09 UTC
Hm, we tested an 18.11.16 package with Kyle's patch but the Editor is now gone:

ReferenceError: tinyMCE is not defined
tinyMCE.baseURL = "/intranet-tmpl/lib/tiny_mce";

Not the result I was hoping for to fix our issues :(
Comment 15 Tomás Cohen Arazi 2020-05-15 14:53:41 UTC
(In reply to Katrin Fischer from comment #14)
> Hm, we tested an 18.11.16 package with Kyle's patch but the Editor is now
> gone:
> 
> ReferenceError: tinyMCE is not defined
> tinyMCE.baseURL = "/intranet-tmpl/lib/tiny_mce";
> 
> Not the result I was hoping for to fix our issues :(

So you rebuilt the package?
Comment 16 Katrin Fischer 2020-05-15 15:13:19 UTC
(In reply to Tomás Cohen Arazi from comment #15)
> (In reply to Katrin Fischer from comment #14)
> > Hm, we tested an 18.11.16 package with Kyle's patch but the Editor is now
> > gone:
> > 
> > ReferenceError: tinyMCE is not defined
> > tinyMCE.baseURL = "/intranet-tmpl/lib/tiny_mce";
> > 
> > Not the result I was hoping for to fix our issues :(
> 
> So you rebuilt the package?

Yes, we built a new package and used it to update an existing 18.11.11 installation. It could be the fix doesn't apply to 18.11 or that someting is not quite right yet.
Comment 17 Owen Leonard 2020-05-15 17:18:20 UTC
(In reply to Kyle M Hall from comment #1)

> When installing the Debian package of Koha, we install the Debian tinymce
> package and symlink over our own tinymce libs that are in the Koha codebase.

Why do we do this?

(In reply to David Cook from comment #5)

> I've often wondered why we used Debian's tinymce package. I think we used to
> do the same thing with the yui Javascript. 

We used to offer the option of loading YUI from Yahoo's CDN.
Comment 18 Katrin Fischer 2020-05-15 18:47:26 UTC
I came to the conclusion that the TinyMCE problem on 18.11.16 is a different problem - I found a workaround I will attach to bug 25519.
Sorry for adding to the confusion here.
Comment 19 Victor Grousset/tuxayo 2020-05-15 22:32:54 UTC
> 1) Install Koha 19.11 from package
> 5) Build a new Koha package with this patch applied, install it

IIUC from various IRC discussions the cleanest way[1] to test this while being simple enough be less like likely to be messed up is:
- make packages with https://hub.docker.com/r/koha/koha-dpkg
- spin a standard Debian (9?) however one wants (container, VM, VPS)
- install the packages following https://wiki.koha-community.org/wiki/Koha_on_Debian

Anything missing?

[1] for someone without experience with building packages and installing Koha from them
Comment 20 Victor Grousset/tuxayo 2020-05-15 23:36:13 UTC
*enough to be less likely to mess things up
Comment 21 David Cook 2020-05-17 23:18:33 UTC
Hoping to work on this more today.
Comment 22 David Cook 2020-05-18 02:45:26 UTC
Getting the following on master:

cp: cannot stat 'debian/tmp/debian/tmp/etc/koha/searchengine': No such file or directory
dh_install: cp -a debian/tmp/debian/tmp/etc/koha/searchengine debian/koha-common//etc/koha/ returned exit code 1
debian/rules:9: recipe for target 'binary' failed

But I don't think it relates to this patch.
Comment 23 David Cook 2020-05-18 02:53:09 UTC
(In reply to David Cook from comment #22)
> Getting the following on master:
> 
> cp: cannot stat 'debian/tmp/debian/tmp/etc/koha/searchengine': No such file
> or directory
> dh_install: cp -a debian/tmp/debian/tmp/etc/koha/searchengine
> debian/koha-common//etc/koha/ returned exit code 1
> debian/rules:9: recipe for target 'binary' failed
> 
> But I don't think it relates to this patch.

Looks like Bug 24735 got rid of etc/searchengine/queryparser.yml, but Bugzilla says that bug was pushed to master at the start of March...

I guess I probably haven't been building master packages since then but https://dashboard.koha-community.org/ says the master build is still passing on D9?
Comment 24 David Cook 2020-05-18 02:57:47 UTC
(In reply to David Cook from comment #23)
> Looks like Bug 24735 got rid of etc/searchengine/queryparser.yml, but
> Bugzilla says that bug was pushed to master at the start of March...
> 
> I guess I probably haven't been building master packages since then but
> https://dashboard.koha-community.org/ says the master build is still passing
> on D9?

(In reply to David Cook from comment #22)
> Getting the following on master:
> 
> cp: cannot stat 'debian/tmp/debian/tmp/etc/koha/searchengine': No such file
> or directory
> dh_install: cp -a debian/tmp/debian/tmp/etc/koha/searchengine
> debian/koha-common//etc/koha/ returned exit code 1
> debian/rules:9: recipe for target 'binary' failed
> 
> But I don't think it relates to this patch.

See Bug 25068
Comment 25 David Cook 2020-05-18 03:28:14 UTC
Ok with Bug 25068's patch and this one (plus a manual update to debian/control), I was able to create a package.

Next, I inspected the files in the package:
dpkg-deb -c koha-common_19.12.00-test+20200518031703.33335a5d-1_all.deb

As I anticipated, it looks like /usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce has been removed (as the debian/rules file removes it).

I'll add a follow-up patch...
Comment 26 David Cook 2020-05-18 03:29:51 UTC
Created attachment 104995 [details] [review]
Bug 25485: Remove all mention of tiny_mce in debian/rules

This patch removes the removal of the following during the
build process:
/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce
Comment 27 David Cook 2020-05-18 05:38:42 UTC
The package now includes the tiny_mce files, but it's not removing the symlink, which is a problem. See Bug 14055 for the YUI related issue.
Comment 28 David Cook 2020-05-18 05:42:50 UTC
Created attachment 104997 [details] [review]
Bug 25485: Remove tiny_mce symlink during pre install of deb package

This patch removes the tiny_mce symlink before Koha package installation
using the same method that we used when we removed the YUI symlink
in 2015.
Comment 29 David Cook 2020-05-18 06:00:51 UTC
Created attachment 104999 [details] [review]
Bug 25485: TinyMCE broken in Debian package installs

When installing the Debian package of Koha, we install the Debian tinymce package and symlink over our own tinymce libs that are in the Koha codebase. This version of tinymce does not have the same files and because of this, tinymce does not get loaded.

We should remove the use of the tinymce Debian package, and just use our own version of TinyMCE.

Test Plan:
1) Install Koha 19.11 from package
2) Browse to the news editor
4) Note the WYSIWYG editor is missing
5) Build a new Koha package with this patch applied, install it
6) Reload the news editor
7) Note the WYSIWYG editor is back!

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 30 David Cook 2020-05-18 06:03:14 UTC
Created attachment 105000 [details] [review]
Bug 25485: TinyMCE broken in Debian package installs

When installing the Debian package of Koha, we install the Debian tinymce package and symlink over our own tinymce libs that are in the Koha codebase. This version of tinymce does not have the same files and because of this, tinymce does not get loaded.

We should remove the use of the tinymce Debian package, and just use our own version of TinyMCE.

Test Plan:
1) Install Koha 19.11 from package
2) Browse to the news editor
4) Note the WYSIWYG editor is missing
5) Build a new Koha package with this patch applied, install it
6) Reload the news editor
7) Note the WYSIWYG editor is back!

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 31 David Cook 2020-05-18 06:03:18 UTC
Created attachment 105001 [details] [review]
Bug 25485: Remove all mention of tiny_mce in debian/rules

This patch removes the removal of the following during the
build process:
/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 32 David Cook 2020-05-18 06:03:22 UTC
Created attachment 105002 [details] [review]
Bug 25485: Remove tiny_mce symlink during pre install of deb package

This patch removes the tiny_mce symlink before Koha package installation
using the same method that we used when we removed the YUI symlink
in 2015.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 33 David Cook 2020-05-18 06:04:53 UTC
Created attachment 105004 [details] [review]
Bug 25485: [Do not push] remove tiny_mce from debian/control for testing
Comment 34 David Cook 2020-05-18 06:07:00 UTC
Ok, using all these patches, you can get the correct behaviour. 

I've signed off Kyle's original patch, and I'm leaving "Needs Signoff" so that someone (maybe Kyle?) can sign off my patches.
Comment 35 David Cook 2020-05-18 07:04:06 UTC
Note that this also affects 19.11 and not just master...
Comment 36 Victor Grousset/tuxayo 2020-05-18 11:29:27 UTC
(In reply to David Cook from comment #23)
> I guess I probably haven't been building master packages since then but
> https://dashboard.koha-community.org/ says the master build is still passing
> on D9?

The CI is not an actual build unfortunately. It's spinning up koha-testing-docker which is a gitified install (if I got the terminology right) like the DevBox and then runs all the tests. But no tests for package build yet.

Fortunately these two projects (including yours :)) are good bases to move it that direction.
https://gitlab.com/tomascohen/koha-dpkg-docker
https://gitlab.com/minusdavid/koha-deb-builder-docker
Comment 37 Chris Cormack 2020-05-18 21:12:31 UTC
Created attachment 105048 [details] [review]
Bug 25485: Remove all mention of tiny_mce in debian/rules

This patch removes the removal of the following during the
build process:
/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 38 Chris Cormack 2020-05-18 21:13:27 UTC
Created attachment 105049 [details] [review]
Bug 25485: Remove tiny_mce symlink during pre install of deb package

This patch removes the tiny_mce symlink before Koha package installation
using the same method that we used when we removed the YUI symlink
in 2015.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 39 Chris Cormack 2020-05-18 21:20:36 UTC
Created attachment 105050 [details] [review]
Bug 25485: [Do not push] remove tiny_mce from debian/control for testing
Comment 40 Tomás Cohen Arazi 2020-05-19 22:44:23 UTC
Created attachment 105127 [details] [review]
Bug 25485: TinyMCE broken in Debian package installs

When installing the Debian package of Koha, we install the Debian tinymce package and symlink over our own tinymce libs that are in the Koha codebase. This version of tinymce does not have the same files and because of this, tinymce does not get loaded.

We should remove the use of the tinymce Debian package, and just use our own version of TinyMCE.

Test Plan:
1) Install Koha 19.11 from package
2) Browse to the news editor
4) Note the WYSIWYG editor is missing
5) Build a new Koha package with this patch applied, install it
6) Reload the news editor
7) Note the WYSIWYG editor is back!

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 41 Tomás Cohen Arazi 2020-05-19 22:44:35 UTC
Created attachment 105128 [details] [review]
Bug 25485: Remove all mention of tiny_mce in debian/rules

This patch removes the removal of the following during the
build process:
/usr/share/koha/intranet/htdocs/intranet-tmpl/lib/tiny_mce

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 42 Tomás Cohen Arazi 2020-05-19 22:44:56 UTC
Created attachment 105130 [details] [review]
Bug 25485: Remove tiny_mce symlink during pre install of deb package

This patch removes the tiny_mce symlink before Koha package installation
using the same method that we used when we removed the YUI symlink
in 2015.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 43 Tomás Cohen Arazi 2020-05-19 22:46:53 UTC
Good job. I tested an upgrade to verify the symlink is removed and doesn't break the upgrade. All good. Thanks and sorry for the delay! Will be trying to test all the packages bugs together tomorrow on a new VPS.
Comment 44 Martin Renvoize 2020-05-20 08:18:32 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 45 Joy Nelson 2020-05-21 17:49:07 UTC
Not sure the dependency affect this in 19.11.x - it wasn't pushed.  This fix for TinyMCE was pushed to 19.11.x for 19.11.06
Comment 46 David Cook 2020-05-21 23:13:50 UTC
(In reply to Joy Nelson from comment #45)
> Not sure the dependency affect this in 19.11.x - it wasn't pushed.  This fix
> for TinyMCE was pushed to 19.11.x for 19.11.06

That's exactly right. Dependency doesn't affect 19.11.x. Great to have this in 19.11.06!
Comment 47 Lucas Gass 2020-05-22 14:51:14 UTC
Correct me if i am wrong but I don't think this issue is present in 19.05.x package installs. Unless I'm told otherwise I won't backport