Skip to content
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

combine_css on XHTML file with unbalanced tags can strip elements #252

Closed
GoogleCodeExporter opened this issue Apr 6, 2015 · 9 comments
Closed

Comments

@GoogleCodeExporter
Copy link

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

What steps will reproduce the problem?
1. When applying the combine_css filter, javascript ( inline and external ) is 
removed from the page

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

0.9.16.9-576

On what operating system?

Linux/CentOS 5.4 64bit

Which version of Apache?

httpd-2.2.3-43.el5.centos.3

Which MPM?

Prefork

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

I have recently upgraded to 0.9.16.9-576, but not exactly sure from which 
version. I noticed that all javascript was stripped, and I started removing 
filters - was just using the core filter set. I have managed to reproduce the 
problem to just cmobine_css being enabled.

I will attach a file which contains the original html without any filters 
applied and one with combine_css applied. There are some spurious changes due 
to the pages being dynamically generated, but the missing javascript is clearly 
shown.

Original issue reported on code.google.com by robert.munteanu on 24 Mar 2011 at 8:36

Attachments:

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

I'm taking a look.  I can see the 

Original comment by jmara...@google.com on 24 Mar 2011 at 9:50

  • Changed state: Started
@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

oops -- hit 'save' too soon.  I can see the broken page with 

http://www.casaretro.com/index.php?ModPagespeedFilters=combine_css

Original comment by jmara...@google.com on 24 Mar 2011 at 9:50

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

I bet we think that the <script> elements are nested inside the <link> that's 
on line 38.

Original comment by morlov...@google.com on 24 Mar 2011 at 9:53

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

morlovich's theory is confirmed.  A quick workaround is to remove the 'doctype' 
tag:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" 
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

mod_pagespeed's parser reads this tag and assumes that the rest of the document 
has xhtml balanced tags, which it doesn't.  The <link> tag on line 38 is 
unclosed.  You could also leave the doctype in but fix all the tags.

But in any case this should be fixed in mod_pagespeed to avoid combining 
malformed <link> tags.

Summary was: combine_css remove javascript from head

Original comment by jmara...@google.com on 25 Mar 2011 at 3:06

  • Changed title: combine_css on XHTML file with unbalanced tags can strip elements
@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Thank you, that does help.

Original comment by robert.munteanu on 25 Mar 2011 at 9:19

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

I have a fix under review.

Original comment by jmara...@google.com on 25 Mar 2011 at 5:17

  • Added labels: release-note
@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

This issue was closed by revision r595.

Original comment by jmara...@google.com on 25 Mar 2011 at 7:55

  • Changed state: Fixed
@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Issue 255 has been merged into this issue.

Original comment by jmara...@google.com on 31 Mar 2011 at 1:23

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Issue 292 has been merged into this issue.

Original comment by jmara...@google.com on 13 May 2011 at 1:16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.