search

crowell / modpagespeed_tmp

Automatically exported from code.google.com/p/modpagespeed
Apache License 2.0
0 stars 0 forks source link

rewrite_javascript breaks Wordpress /wp-admin/ pages with certain common WP plugins #675

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
This has yet to be reproduced locally, the bug is described in these pages:

http://wordpress.org/support/topic/35-rc3-new-media-uploader-doesnt-work-with-go
ogle-mod_pagespeed <-- more illegal character.  The file has a BOM in it, 
perhaps MPS is serving something other than UTF-8?  Perhaps we need to 
propagate the charset?
https://groups.google.com/forum/#!msg/mod-pagespeed-discuss/r6lcoseTJks/IrQD04sQ
0PcJ  <-- add media button didn't pop up the iframe overlay.  Firebug showed a 
malformed unicode character escape sequence but I have no idea what file it was 
in.
http://wordpress.org/support/topic/add-media-button-stopped-working
http://wordpress.org/support/topic/yet-another-35-add-media-button-failure
http://osdir.com/ml/mod_pagespeed/2011-04/msg00009.html
https://groups.google.com/forum/?fromgroups=#!topic/mod-pagespeed-discuss/XpRASA
KbELA

The symptom is that some javascript does not run on people's pages such as the 
add media page in the panel. When rewrite_javascript is disabled it works fine. 
An alternate temporary fix is to disable wordpress js concatenation.

What I believe is happening is that the concatenated js has UTF-8 BOM markers 
in the middle (at the insertion point of a new file) and mod_pagespeed is 
somehow not propagating the correct UTF-8 charset.  When the BOM markers are 
seen with the wrong given charset, syntax errors are produced.

When wordpress concatenation is disabled, the BOMs would be in their proper 
position, at the beginning of the individual js files, which might help the 
parser even if mod_pagespeed fails to propagate the charset correctly.

Original issue reported on code.google.com by jkar...@google.com on 17 Apr 2013 at 5:48

GoogleCodeExporter commented 9 years ago 
There are two possible solutions.

1) Remove byte order markers in positions not at the start of the javascripts 
in the JavaScript minifier as ecmascript says that these should be treated like 
whitespace anyway.

2) Propagate the content-type charset header from the server into rewritten 
resources.

Or we could do both.  But the second option is a more general solution, so we 
should at least do two and maybe one as well.

Original comment by jkar...@google.com on 17 Apr 2013 at 6:46

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
On second thought solution 1 might change the behavior of the site, so we 
should not do that.

Original comment by jkar...@google.com on 17 Apr 2013 at 7:07

GoogleCodeExporter commented 9 years ago 
Out of curiosity, why would solution #1 change the behavior?

Original comment by jmara...@google.com on 17 Apr 2013 at 7:32

GoogleCodeExporter commented 9 years ago 
If there are utf-8 BOMs in the javascript and utf-8 isn't set as the charset 
then the page might break in a browser.  If mod_pagespeed removes the BOMs then 
the page would no longer break.  We'd be 'fixing' people's pages, which is not 
the intent.

Original comment by jkar...@google.com on 18 Apr 2013 at 10:41

GoogleCodeExporter commented 9 years ago 
Note that the file that was giving problems, jquery.query.js, has recently had 
its BOM removed for exactly this reason:

http://core.trac.wordpress.org/ticket/23315

Original comment by jkar...@google.com on 18 Apr 2013 at 11:51

GoogleCodeExporter commented 9 years ago 
mod_pagespeed has been propagating charset since at least 0.10.22.4-r1633, but 
I can confirm that it did not in 0.9.18.7 which is what 
https://groups.google.com/forum/#!msg/mod-pagespeed-discuss/r6lcoseTJks/IrQD04sQ
0PcJ is using.

Original comment by jkar...@google.com on 18 Apr 2013 at 1:30

GoogleCodeExporter commented 9 years ago 
So this is apparently not the problem for 
http://wordpress.org/support/topic/35-rc3-new-media-uploader-doesnt-work-with-go
ogle-mod_pagespeed who is running 1.4.26.2-2759 on his site.

Original comment by jkar...@google.com on 18 Apr 2013 at 1:40

GoogleCodeExporter commented 9 years ago 
That topic was raised before we released 1.4.  My comment on that site is the 
first made since we released 1.4.

Original comment by jmara...@google.com on 18 Apr 2013 at 1:47

GoogleCodeExporter commented 9 years ago 
But he said in the first post that he was running stable, which should be >= 
0.10-beta right?

Original comment by jkar...@google.com on 18 Apr 2013 at 1:53

GoogleCodeExporter commented 9 years ago 
Good point, seems like branch 22 was the first one with the encoding change...

Original comment by morlov...@google.com on 18 Apr 2013 at 2:03

GoogleCodeExporter commented 9 years ago 
I was able to force wordpress to break by doing the following:

0) run old mod_pagespeed (0.9.18.7) with default rewriting rules.
1) run wordpress-3.2.1 (it lets you change the default charset) and set the 
wordpress default charset to koi8-r in admin -> settings -> reading. (it 
defaults to utf-8 which ignores the BOMs)
2) add the line "wp_enqueue_script('jquery-query'); around line 24 of 
wpadmin/media-upload.php so that it concatenates the offending file (this isn't 
loaded by default, presumably some plugin which I cannot find would cause this 
to happen as it shows up in the URLs of the reported problems).
3) Now browse to media->new media on the console and you'll see the syntax 
error in the console, though it doesn't appear to break anything on the page.

So could this be the problem?  Did people change their charset from UTF-8?  
That's harder to do in more recent wordpress releases but maybe if you just 
keep upgrading the setting sticks around.

Original comment by jkar...@google.com on 18 Apr 2013 at 3:27

GoogleCodeExporter commented 9 years ago 
The charset responses for html from the sites in question are both UTF-8 and 
not alternates, so that's unlikely to be the problem.

Original comment by jkar...@google.com on 18 Apr 2013 at 3:45

GoogleCodeExporter commented 9 years ago 
The problem wound up being an issue with the javascript lexer in js_minify.  
Specifically, when trying to determine if a '/' is a divide symbol or the start 
of a regex, it could get confused, causing the rest of the javascript to be 
incorrectly minified.

The specific problem for Wordpress was in file underscore.min.js and is used in 
the AddMEdia button on the page to add a new post. The script contains the 
line, 'typeof /./', where the first '/' was seen as a divide instead of the 
start of a regex.

Original comment by jkar...@google.com on 23 Apr 2013 at 5:25

GoogleCodeExporter commented 9 years ago

Original comment by jkar...@google.com on 25 Apr 2013 at 10:46

GoogleCodeExporter commented 9 years ago

Original comment by jkar...@google.com on 25 Apr 2013 at 12:07

GoogleCodeExporter commented 9 years ago

Original comment by jmara...@google.com on 25 Apr 2013 at 1:14

GoogleCodeExporter commented 9 years ago

Original comment by sligocki@google.com on 24 Jun 2013 at 2:11