New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

js_tinyMCE breaks when it's name is changed #38

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 33 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

What steps will reproduce the problem?
1. install mod_pagespeed
2. add this line to pagespeed.conf
    ModPagespeedEnableFilters rewrite_javascript
3. visit your site

What version of the product are you using (please check X-Mod-Pagespeed
header)?
revisiton 147

On what operating system?
archlinux

Which version of Apache?
2.2.17

Please provide any additional information below, especially a URL or an
HTML file that exhibits the problem.

I get these errors in firebug console:
invalid increment operand
[Break on this error] 
=this.cookie||(this.cookie=this.option...on(){this.lis.filter(".ui-state-proces
jm.67e...5395.js (line 21)
$(".calendar input").datepicker is not a function
[Break on this error] 
$('#fancybox-outer').css({'-moz-box-sh...on(){if(this.last_change===undefined)
jm.b6a...3360.js (line 61)

It seems the problem is in minified jquery-ui.js.


Original issue reported on code.google.com by jan.h....@gmail.com on 7 Nov 2010 at 9:27

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

here is the compressed responsed jquery-ui.js 
(jm.67e3c8adb4006404dca4983ca510f741.jquery-ui-1,o8,o1,ocustom,omin,l?1287375395
.js) I got.

Original comment by jan.h....@gmail.com on 7 Nov 2010 at 9:30

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the bug report.  Can you also provide a URL to your site?

We have seen other reports of rewrite_javascript breaking pages and we have a 
fix almost ready.  But we'd love to test it on your site to make sure it's 
completely functional.

Original comment by jmara...@google.com on 7 Nov 2010 at 12:58

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I'm sorry but I can't provide a url since it's a internal CMS for our client. I 
can test the fix asap and report here once it's released.

Thanks.

Original comment by jan.h....@gmail.com on 7 Nov 2010 at 1:03

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

No problem.  Can you attach the jquery-ui.js prior to it being minified by 
mod_pagespeed?  That would be sufficient.  (I know we can get jquery from its 
origin but I we want to tet the exact bytes that failed for you, if possible).

Original comment by jmara...@google.com on 7 Nov 2010 at 1:37

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Here it is :)

Original comment by jan.h....@gmail.com on 7 Nov 2010 at 3:38

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

awesome.  thanks.  we are moving as fast as we can to resolve the 
rewrite_javascript issues; and we'll be sure that this one is still loading 
after minification.

Original comment by jmara...@google.com on 7 Nov 2010 at 10:43

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 58 has been merged into this issue.

Original comment by jmara...@google.com on 9 Nov 2010 at 2:47

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

JQuery TinyMCE also suffers from this same issue.

Original comment by rwap.services on 10 Nov 2010 at 7:30

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Basically, the issue seems mainly related to javascripts which do not always 
declare the functions before they are called in the code and will quite often 
be picked up as issues in JSLint.  

Perhaps what is needed is a pre-processor which can check for this problem and 
if it exists, then automatically preclude the files from being rewritten, 
unless there is a work around :)

Original comment by rwap.services on 11 Nov 2010 at 12:06

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

The javascript minifier has been replaced with one that appears to be much more 
functional across a wide range of web sites.


Original comment by jmara...@google.com on 13 Nov 2010 at 12:50

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Is it enabled by default now?

Original comment by jan.h....@gmail.com on 13 Nov 2010 at 2:36

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

js_tinyMCE still does not work with the new javascript minifier - I wonder if 
there is a way of checking for the issue referred to in my comment 9 above and 
adding the javascript to a blacklist if this is found to be an issue?

Original comment by sexy.ric...@googlemail.com on 17 Nov 2010 at 8:24

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

It is still not enabled by default.

We will take a look at js_tinymce and see what the problem is.

Original comment by jmara...@google.com on 17 Nov 2010 at 12:26

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Could you please attach the specific version of js_tinyMCE that is still 
breaking for you?  (Ideally, both the original file and the output that 
rewrite_javascript is producing.)

Original comment by mdste...@google.com on 17 Nov 2010 at 4:47

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I can reproduce the problem using the latest version of tinyMCE and this simple 
form



Original comment by hhub...@gmail.com on 17 Nov 2010 at 10:51

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Please attach your version of tiny_mce.js, while searching we found several 
different versions.

Original comment by sligocki@google.com on 17 Nov 2010 at 10:57

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Version 3.3.9.2 from http://tinymce.moxiecode.com/download.php

I also pulled the trunk and tried it on 32bit Ubuntu 10.04.1.  Same results.

Thanks for any help.

Original comment by hhub...@gmail.com on 18 Nov 2010 at 2:26

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks; I am able to reproduce this problem now.  I will start work on a fix.

Original comment by mdste...@google.com on 19 Nov 2010 at 9:50

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 20 Nov 2010 at 12:23

  • Changed title: js_tinyMCE doesn't work with rewrite_javascript enabled
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

The same issue occurs with CKeditor WYSIWYG editor too !

Original comment by ayma...@gmail.com on 20 Nov 2010 at 10:07

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Problem also happens with animatedcollapse.js - see attached.

I do think that the main issue here relates to files that do not define 
functions before they are called (and show errors in JSLint).

Original comment by rwap.services on 20 Nov 2010 at 10:30

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

r is not a constructor in tinymce. help!!! please
file in html:
tiny_mce/ce.10ab9d53eeee67e965758659fed1f8aa.tiny_mce,l.js

Thanks for any help.

Original comment by anuncios...@gmail.com on 24 Nov 2010 at 2:42

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I believe I may have diagnosed the problem.

The problem seems not to be with the Javascript rewriting per se, but with the 
fact that the file is being renamed.  If you change the name of tiny_mce.js to, 
say, tiny_mce.orig.js, it no longer works; conversely, if you minify it 
manually with mod_pagespeed's Javascript minifier and keep the file name the 
same, it works fine.  I _think_ the reason for this is that tinyMCE is 
searching the HTML for its own URL, and if you change the filename, it may not 
be able to find it.

Unfortunately, there's not a lot we can do to detect when this might be an 
issue for a particular script.  We are working on providing a way to tell 
mod_pagespeed not to rewrite certain files/directories, which will help.

I'll try to look into some of the other scripts that folks have mentioned here 
besides tinyMCE to verify that there aren't additional problems with our 
Javascript rewriting.

Original comment by mdste...@google.com on 24 Nov 2010 at 4:07

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Ah, this is a known problem that all the rewriter suffer from. Any change to 
the page (especially renaming files) can potentially break Javascript because 
it can be extremely introspective. It is, in general, not possible to figure 
out whether Javascript will do this.

However, we are looking into allowing users to blacklist certain files so that 
they won't be rewritten. Would that be a reasonable solution for you?

Original comment by sligocki@google.com on 24 Nov 2010 at 4:23

  • Changed title: js_tinyMCE breaks when it's name is changed
  • Changed state: New
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

The ability to blacklist certain files is definitely a reasonable solution - 
perhaps with the ability to process files through the minifier manually, so 
that the minified version can still be stored on the server (overwriting the 
original file) at the user's discretion.

Original comment by rwap.services on 24 Nov 2010 at 11:27

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Is there a workaround while we wait? I tried disabling filters and nothing 
seemed to work short of turning off page speed completely.

Original comment by bcrui...@gmail.com on 3 Dec 2010 at 6:12

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

The only workaround I can think of until our next release is to turn off 
rewrite_javascript (which is off by default) and extend_cache (which is on by 
default).  Please let me know if that doesn't work.

But soon you will be able to instead add this directive:

  ModPagespeedDisallow */jquery-ui-1.8.2.custom.min.js
  ModPagespeedDisallow */js_tinyMCE.js

Stay tuned!

Original comment by jmara...@google.com on 3 Dec 2010 at 6:22

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Added "ModPagespeedDisableFilters extend_cache" as a workaround and it worked! 

Thanks so much for your awesome work!

Original comment by bcrui...@gmail.com on 3 Dec 2010 at 7:18

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This is fixed in SVN now so we are marking it fixed in preparation for the 
branch.

Of course the issue is cannot be completely fixed, but at least there is a 
surgical workaround now, which we I have put into the FAQ: 
http://code.google.com/p/modpagespeed/wiki/FAQ?ts=1291409653&updated=FAQ

Original comment by jmara...@google.com on 3 Dec 2010 at 8:57

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 3 Dec 2010 at 8:58

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 142 has been merged into this issue.

Original comment by sligocki@google.com on 22 Dec 2010 at 6:00

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 391 has been merged into this issue.

Original comment by sligocki@google.com on 23 Feb 2012 at 6:23

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 142 has been merged into this issue.

Original comment by morlov...@google.com on 14 Aug 2013 at 2:59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment