PiRSquared17 / activescaffold

Automatically exported from code.google.com/p/activescaffold
MIT License
0 stars 0 forks source link

IE stylesheets gets included no matter what browser user is on #750

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
As of Rails 2.3.8, the html IF tags that are supposed to only include the 
Active Scaffold stylesheet 
if the user is on IE, doesn't seem to work anymore. The html gets escaped and 
the result is the 
<!--[if IE] tags getting output as text on the page, and the IE stylesheet 
getting included no 
matter what. This seems to be caused by the method "active_scaffolds_includes" 
in 
view_helpers.rb (around line 100 to 114). I suggest using the built in user 
agent detection 
instead of relying on html if tags. A suggested fix to the method could be:

      # easy way to include ActiveScaffold assets
      def active_scaffold_includes(*args)
        frontend = args.first.is_a?(Symbol) ? args.shift : :default
        options = args.first.is_a?(Hash) ? args.shift : {}
        js = javascript_include_tag(*active_scaffold_javascripts(frontend).push(options))

        css = stylesheet_link_tag(*active_scaffold_stylesheets(frontend).push(options))
        ie_css = stylesheet_link_tag(*active_scaffold_ie_stylesheets(frontend).push(options))
        ie = ie_css + "\n" if request.env['HTTP_USER_AGENT'] =~ /MSIE/
        js + "\n" + css + "\n" + ie
      end

Then again, I don't know if this is actually just my setup being stupid in some 
way, but I just felt 
like using the "<!-- [if IE]" tags seemed a bit deprecated ;)

- Mark

Original issue reported on code.google.com by markqv...@gmail.com on 26 May 2010 at 5:05

GoogleCodeExporter commented 9 years ago
I can't reproduce it, are you using rails_xss plugin?

I prefer to use conditional comments, user agent can be faked, and it's easy to 
set
different css for different IE versions with conditional comments (when some IE
version works right)

Original comment by sergio.c...@gmail.com on 26 May 2010 at 8:23

GoogleCodeExporter commented 9 years ago

Original comment by sergio.c...@gmail.com on 31 May 2010 at 7:56

GoogleCodeExporter commented 9 years ago
I can reproduce the problem with brand-new project on rails 2.3.8:

I think the issue is that the comment that's supposed to do the "IE-only" magic 
gets 
escaped by Rail's new HTML-safe magic:

The HTML source looks like this:
---
<!--[if IE]><link href="/stylesheets/active_scaffold/default/stylesheet-ie.css?
1275467413" media="screen" rel="stylesheet" type="text/css" /><![endif]-->
---

In Rails 2.3.5 the line looks like this:
---
<!--[if IE]><link href="/stylesheets/active_scaffold/default/stylesheet-ie.css?
1275468532" media="screen" rel="stylesheet" type="text/css" /><![endif]--> 
---

This may be the reason:
http://github.com/rails/rails/commit/9415935902f120a9bac0bfce7129725a0db38ed3

I think this may break other things as well, so it should probably be addressed.

Original comment by wund...@gmail.com on 2 Jun 2010 at 8:50

GoogleCodeExporter commented 9 years ago
Are you using rails_xss plugin? rails 2.3.8 doesn't force escaping, has support 
to
use rails_xss plugin to force escaping like rails 3.

ActiveScaffold doesn't support rails_xss, but now it's easy to support it 
without
forcing to use it, so we can add support for rails_xss, but we need to know if
you're using rails_xss plugin

Original comment by sergio.c...@gmail.com on 2 Jun 2010 at 11:32

GoogleCodeExporter commented 9 years ago
Sorry for late response here. I'm not using rails_xss in my project.

Original comment by markqv...@gmail.com on 2 Jun 2010 at 11:39

GoogleCodeExporter commented 9 years ago
No, I am not using rails_xss, just plain 2.3.8. It seems they still have not 
got it 
quite right in 2.3.8, concatting strings with '+' still causes unwanted 
escaping. :(

However, I just noticed that the master branch already has a fix (it's patch 
3551c4, 
http://github.com/activescaffold/active_scaffold/commit/3551c40c762c1c7a1967b35c
98efb1
7ccb1bb6a2). So switching to 'master' instead of rails 2.3 fixes the problem. 
Maybe 
that patch should also be merged into the 2.3 branch.

Original comment by wund...@gmail.com on 2 Jun 2010 at 12:04

GoogleCodeExporter commented 9 years ago
Oh, I thought it was master branch, so I couldn't reproduce it. It's backported 
now

Original comment by sergio.c...@gmail.com on 2 Jun 2010 at 2:44