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

Support charset meta tag. #3877

Closed
wants to merge 1 commit into from
Closed

Conversation

kennyj
Copy link
Contributor

@kennyj kennyj commented Dec 6, 2011

I know GH #3539 and #149.
But I want to propose it again, because I have another motivation.

I'm Japanese and resident in the multibyte world.
In my experience, if we don't write charset meta tag, we've got trouble in terms of the next.

If we save a page as html file.

When we open the html file, we'll miss a charset .

@josevalim
Copy link
Contributor

This is a very good point but I would like a simpler implementation. Maybe we should just check response.charset given #3878 is merged.

@wycats
Copy link
Member

wycats commented Dec 6, 2011

Why do you need the charset tag when we are already sending the Content-Type header containing a charset?

@josevalim
Copy link
Contributor

He just explained: "If we save a page as html file. When we open the html file, we'll miss a charset."

@kennyj
Copy link
Contributor Author

kennyj commented Dec 7, 2011

@josevalim

I think that response.charset is empty in the charset_meta_tag rendering time
except we explicitly assign response.charset (a.k.a. normal case)

Thus, I used charset argument, Content-Type, response.charset and AD::Response.default_charset in above patch.

But, certainly, my implementation is a little complexity.
For example, do we stop arguement and Content-Type support ?

      def charset_meta_tag
        tag('meta', :charset => (response.charset || response.class.default_charset)).html_safe
      end

@josevalim
Copy link
Contributor

IMHO:

  def charset_meta_tag(arg=nil)
    tag('meta', :charset => (arg || response.charset || response.class.default_charset)).html_safe
  end

@kennyj
Copy link
Contributor Author

kennyj commented Dec 8, 2011

@josevalim OK!

I'll fix and squash it after work.

Just FYI:
In my past java experience, and in rare cases, I must have to be different for response header charset and meta tag's charset.

Java runtime know Windows-31J encoding, and this encoding is necessary for properly character conversion.
But, it seems that IE6 don't know Windows-31J. We must set Shift_JIS (that is like to Windows-31J) to meta tag's charset ;-)

@josevalim
Copy link
Contributor

Yeah, if this is the case, you should set response.charset then, and you
will probably be fine. Confirm/deny?

@kennyj
Copy link
Contributor Author

kennyj commented Dec 8, 2011

Confirm. It's work fine (I set response.charset in a controller... My interpretation of the English, right?)

I fixed and squished.

@kennyj
Copy link
Contributor Author

kennyj commented Feb 27, 2012

This method is usefull for me, but not for majority (I guess)
I'm closing this PR :)

@kennyj kennyj closed this Feb 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants