Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#21751 closed defect (bug) (fixed)

Twenty Twelve: allow translators to disable Open Sans custom font

Reported by: pavelevap's profile pavelevap Owned by: lancewillett's profile lancewillett
Milestone: 3.5 Priority: normal
Severity: major Version: 3.5
Component: Bundled Theme Keywords: has-patch commit
Focuses: Cc:

Description

Open Sans probably does not have support for Czech characters, so after activating Twenty Twelve (latest trunk), all characters like ž, š, č, etc. are not visible on website.

There should be any settings for translators to disable this font by default or load similar supported font?

Related: #21694

Attachments (12)

21751.diff (1.3 KB) - added by obenland 12 years ago.
21751.2.diff (1.6 KB) - added by SergeyBiryukov 12 years ago.
21751.opera.png (20.1 KB) - added by SergeyBiryukov 12 years ago.
21751.chrome.jpg (30.3 KB) - added by pavelevap 12 years ago.
21751.chrome.png (31.5 KB) - added by SergeyBiryukov 12 years ago.
21751.opera.2.png (41.5 KB) - added by SergeyBiryukov 12 years ago.
21751.open-sans-subsets.diff (1.7 KB) - added by obenland 12 years ago.
open-sans-japanese-vista.jpg (73.7 KB) - added by Nao 12 years ago.
Google Chrome Web Fonts bug
21751.cyrillic-arial.png (88.6 KB) - added by SergeyBiryukov 12 years ago.
21751.cyrillic-open-sans.png (87.3 KB) - added by SergeyBiryukov 12 years ago.
21751.3.diff (1.4 KB) - added by obenland 12 years ago.
21751.4.diff (1.4 KB) - added by lancewillett 12 years ago.
Better off logic, no-subset as default extra subset string

Download all attachments as: .zip

Change History (63)

#1 @SergeyBiryukov
12 years ago

  • Milestone changed from Awaiting Review to 3.5

#2 @nacin
12 years ago

  • Severity changed from normal to major

#3 @obenland
12 years ago

  • Summary changed from Open Sans does not support all languages to Twenty Twelve: Open Sans does not support all languages

@obenland
12 years ago

#4 follow-up: @obenland
12 years ago

21751.diff is modeled after the word count switch. Not the most elegant solution, but might just be something if all else fails.

#5 in reply to: ↑ 4 @SergeyBiryukov
12 years ago

  • Keywords has-patch added

Replying to obenland:

21751.diff is modeled after the word count switch.

Was thinking along the same lines.

21751.2.diff is an attempt to make the string itself more meaningful. (In lines 109–112, spaces in the beginning were removed.)

Last edited 12 years ago by SergeyBiryukov (previous) (diff)

#6 @lancewillett
12 years ago

Open Sans probably does not have support for Czech characters

"Probably"—did you check the browser character chart to be sure? http://www.google.com/webfonts/specimen/Open+Sans.

I see ž there in both lowercase and capital versions.

This is one reason why we chose the font, it has a very robust character set support.

This version contains the complete 897 character set, which includes the standard ISO Latin 1, Latin CE, Greek and Cyrillic character sets.

#7 @lancewillett
12 years ago

@pavelevap, can you please provide more debugging information?

WP version number, theme version number, OS version, browser version, language settings, database setup, host -- all that info can help other folks try to find and repeat issues. Thanks!

#8 follow-up: @lancewillett
12 years ago

I put up a quick test page: http://lancetest24.wordpress.com/2012/08/31/lorem-is-dead-baby/.

I'll add more later, just wanted to show that Česky characters are appearing correctly in this environment, WP.com running latest core trunk with Twenty Twelve version .9. Tested in Chrome 22.x and Safari 6.0 on Mac OS X 10.7.4.

I'll try the test post on a self-hosted test site also.

#9 @lancewillett
12 years ago

http://themebuster.wordpress.net/open-sans-in-various-languages/ is running Twenty Twelve .9 as of r21685 and WordPress 3.5-alpha-21684. Self-hosted install.

#10 in reply to: ↑ 8 ; follow-up: @SergeyBiryukov
12 years ago

Replying to lancewillett:

I put up a quick test page: http://lancetest24.wordpress.com/2012/08/31/lorem-is-dead-baby/.

Tested in Firefox 15, Chrome 21, IE 7, IE 8, Opera 12 on Windows XP.

Looks good in Firefox and Chrome. In IE and Opera, the characters are still visible, but some are too small and misaligned (21751.opera.png), so it might make sense for theme translators to disable the font in that case.

Last edited 12 years ago by SergeyBiryukov (previous) (diff)

#11 @pavelevap
12 years ago

Strange, see attached screenshot (Chrome 21). Using Windows Vista, latest trunk.

#12 follow-up: @SergeyBiryukov
12 years ago

Reproduced in Chrome on Vista: 21751.chrome.png. Looks good in Windows 7 though.

#13 follow-up: @pavelevap
12 years ago

Yes, confirmed on another PC. This problem is related only to combination of Chrome and Windows Vista. IE and Firefox works well even if there are small glitches with some special characters (similar as opera screenshot)...

#14 in reply to: ↑ 13 @SergeyBiryukov
12 years ago

Cyrillic letters are also missing in that combination, so now I'm considering disabling Open Sans for Russian package as well.

#15 in reply to: ↑ 10 @SergeyBiryukov
12 years ago

Replying to SergeyBiryukov:

In IE and Opera, the characters are still visible, but some are too small and misaligned

Related: #21775

#16 @lancewillett
12 years ago

Re: the patches to disable the font from a translation file. Is that the most elegant solution?

I like it as a way for translators to disable for their language, but it seems a tiny bit complex.

#17 @lancewillett
12 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In [21882]:

Twenty Twelve: allow Open Sans to be disabled per language, since it does not support all languages, and some odd combinations of OS and browser versions don't support any non-Latin language very well.

Props obenland and SergeyBiryukov for patches, fixes #21751.

#18 follow-up: @nacin
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm just going to clean up [21882] a little.

#19 @SergeyBiryukov
12 years ago

#21775 was marked as a duplicate.

#20 in reply to: ↑ 18 @lancewillett
12 years ago

Replying to nacin:

I'm just going to clean up [21882] a little.

I'm open to suggestions to make it better -- and instead of you just doing it, I can learn for next time.

#21 @thomask
12 years ago

Open Sans actually supports Czech (and many others) languages, the problem is, that you are using only default (west) subset of google font - look at point 2 here http://www.google.com/webfonts#QuickUsePlace:quickUse/Family:Open+Sans

I wrote about some solutions (and one other current problem) on #21972

#22 in reply to: ↑ 12 @SergeyBiryukov
12 years ago

Replying to SergeyBiryukov:

Reproduced in Chrome on Vista: 21751.chrome.png. Looks good in Windows 7 though.

Also reproduced missing characters in Opera on Windows 7: 21751.opera.2.png. Related: #21775.

#23 @nacin
12 years ago

#21972 was marked as a duplicate.

#24 follow-up: @pavelevap
12 years ago

Yes, thomask is right.

Using following code in header.php works well for Czech.

<link href='http://fonts.googleapis.com/css?family=Open+Sans&subset=latin,latin-ext' rel='stylesheet' type='text/css'>

Maybe subset parameter could be changed by translators?

I also tried to use subset parameter in functions.php for wp_enqueue_style(), but font was not loaded. I am not sure why...

#25 in reply to: ↑ 24 @lancewillett
12 years ago

Replying to pavelevap:

Yes, thomask is right.

Hmm, I don't think so. Loading the font without a character set loads all the character sets, not just Western.

#26 @pavelevap
12 years ago

Strange... So, why this works for me:

<link href='http://fonts.googleapis.com/css?family=Open+Sans&subset=latin,latin-ext' rel='stylesheet' type='text/css'>

and this not:

<link href='http://fonts.googleapis.com/css?family=Open+Sans' rel='stylesheet' type='text/css'>

#27 @lancewillett
12 years ago

Interesting -- I must have read the Google Fonts API incorrectly. I thought in order to only load certain character subsets you would use subset to specify only the ones you wanted -- and leaving it off would load all.

Options are latin, latin-ext, cyrillic, cyrillic-ext, greek, greek-ext, vietnamese.

I think for "en" cases where the setting is English, we should load Latin and Latin Extended.

Then, we have options on how to treat non-English languages by loading extra character sets.

  1. Try to guess by the site's current language setting, and load that extra character set if it's appropriate.
  2. Let translators specify the character subset, and load it if it matches a whitelist we provide (from Open Sans page: http://www.google.com/webfonts#QuickUsePlace:quickUse/Family:Open+Sans)
  3. Load everything for everyone (could be slow for performance)
Last edited 12 years ago by lancewillett (previous) (diff)

#28 @lancewillett
12 years ago

Discussing in IRC with SergeyBiryukov and obenland today. Obenland is going to work up a patch for switching in the extra subsets per option 2 above.

Based on the assumption that everyone regardless will get Latin and Latin Extended.

#29 @Nao
12 years ago

  • Version changed from 3.4.1 to trunk

FYI: Japanese user reported that after adding the patch Chrome Vista still didn't display Japanese fonts. Open Sans works okay on other common browsers - this seems to be a general Google Web Fonts bug in Chrome on Vista.

The market share of Chrome on Vista in Japan is 10% or so (varies on reports) and Chrome by itself (across all platforms) 14% - The Japanese package team will probably decide to just provide information on how to disable Open Sans.

@Nao
12 years ago

Google Chrome Web Fonts bug

#30 follow-up: @thomask
12 years ago

And BTW i would like to point to one other relevant problem with this implementation of Google Web Fonts - you are using simple Standard way - just linking the CSS in head. It is the easiest method, but also the slowest - most browsers will first display the web WITHOUT fonts and after they download the font, they redraw the scren.

Much better would be using javascript implementation, which would load the font after page load - if user do not have the font in system, the page is first loaded with second font in a row, so the user can start read a page few momments before. For technical details how to do it see https://developers.google.com/webfonts/docs/webfont_loader

#31 @thomask
12 years ago

Nao: IMO your problem with Chrome is the exact example what i was described bellow - Chrome is one of the browsers (if i remember right, all except firefox), which display no fonts at all when it does not download a font - on firefox you see the standard system font. When it sould be implemented with the javascript way, i think this problem would not appear.

#32 in reply to: ↑ 30 @lancewillett
12 years ago

Replying to thomask:

Much better would be using javascript implementation, which would load the font after page load - if user do not have the font in system, the page is first loaded with second font in a row, so the user can start read a page few momments before. For technical details how to do it see https://developers.google.com/webfonts/docs/webfont_loader

Noted, we discussed this back in the beginning of development and opted to go with the default CSS link in head.

#33 @lancewillett
12 years ago

  • Summary changed from Twenty Twelve: Open Sans does not support all languages to Twenty Twelve: allow translators to disable Open Sans custom font

#34 @lancewillett
12 years ago

After a bunch more testing and research, it appears the subset parameter has no effect on the language character support. It works with out it and with it.

I think we should go with the code we have now in place, allowing translators to turn it on or off for their language.

#35 @lancewillett
12 years ago

I contacted Google Webfonts via [this link https://docs.google.com/spreadsheet/viewform?formkey=dDNxOUxqdWJONnczY0NCMENIMmpjbkE6MQ#gid=0] to ask if the lack of support in Google Chrome in Windows Vista is a known issue, and asking them about the subset usage (which appears to not have any effect).

Version 0, edited 12 years ago by lancewillett (next)

#36 @lancewillett
12 years ago

Next steps to close this ticket:

  1. Translators: please test with your language. Note the Windows Vista + Google Chrome issue mentioned above.
  2. @nacin: were you going to do some code cleanup? Please let me know.

#37 @thomask
12 years ago

BTW is there any easy way, how we can use latest nightly just for templates, not for whole core? Or do i have to manualy download whole latest zip and replace the old parts? It would be nice if it would be part of WordPress Beta Tester Plugin

#38 @SergeyBiryukov
12 years ago

Replying to lancewillett:

After a bunch more testing and research, it appears the subset parameter has no effect on the language character support. It works with out it and with it.

From my testing, it does make a difference. I've got consistent results on Firefox 15, Chrome 21 and 23, IE 8 and 9, Opera 12, on both Windows XP and Windows 7.

  1. On initial page load, all characters are visible, although Cyrillic characters are displayed in Arial: 21751.cyrillic-arial.png (also ticket:21775:21775.content.png in Opera).
  2. Adding &subset=latin,cyrillic to the wp_enqueue_style() call fixes that in all browsers, *even in Chrome on Vista*: 21751.cyrillic-open-sans.png.

So it seems that 21751.open-sans-subsets.diff would be helpful after all.

P.S. The only weird thing I've noticed is that on Windows 7, Opera doesn't fall back to Arial if I remove &subset=latin,cyrillic after step 2. Cyrillic characters become invisible (21751.opera.2.png) until the browser is restarted (clearing cache doesn't help). Then it falls back to Arial again.

#39 @pavelevap
12 years ago

I agree with Sergey, using subset parameter works almost without problems.

#40 @lancewillett
12 years ago

@SergeyBiryukov Thanks for all the testing notes. I'll get in the subset switching today based on 21751.open-sans-subsets.diff.

#41 @lancewillett
12 years ago

In [22020]:

Twenty Twelve: allow translators to load extra character subsets for Open Sans font, props obenland. See #21751.

#42 @lancewillett
12 years ago

  • Keywords close added; has-patch removed

#43 @nacin
12 years ago

In [22049]:

Simplify the Open Sans translator voodoo. see #21751.

#44 @TobiasBg
12 years ago

This logic seems wrong:

if ( 'off' == _x( 'on', 'Open Sans font: on or off', 'twentytwelve' ) ) {

That would result in Open Sans only being included, if 'on' is translated to 'off'.

#45 @pavelevap
12 years ago

Yes, I noticed the same thing as TobiasBg while testing...

Also, I do not think that translating empty space is the best idea. Maybe "latin,latin-ext" could be "translated"? It is much more intuitive for translators... But on the other hand, "latin,latin-ext" have to be always there...

@obenland
12 years ago

#46 @obenland
12 years ago

21751.3.diff adds a missing 'i' in 'cyrillic', removes an 's' in $subsets when $subset was meant, changes conditional to yoda style, and changes 'off' to 'on'.

Last edited 12 years ago by obenland (previous) (diff)

#47 @SergeyBiryukov
12 years ago

  • Keywords has-patch commit added; close removed

#48 follow-up: @lancewillett
12 years ago

Thanks for the notes and patch — Nacin's changes in [22049] did indeed break this as the logic is incorrect.

Testing more with the subsets — do you think latin,latin-ext should be the translatable string instead of empty? For those languages with the extra subsets it is more likely an "or" case instead of "and" for their subset plus Latin.

#49 in reply to: ↑ 48 @obenland
12 years ago

Replying to lancewillett:

Testing more with the subsets — do you think latin,latin-ext should be the translatable string instead of empty? For those languages with the extra subsets it is more likely an "or" case instead of "and" for their subset plus Latin.

TBH, I don't know enough about the languages that use the extra character sets to add insight here. :) I can imagine them needing latin, to display text that is not in their language, like in quotes etc.

I don't think we should have an empty string for translation, though. If we don't end up making the subset itself translatable, maybe 'subset'? Or 'no subset'?

#50 @nacin
12 years ago

That was supposed to be 'off' !==. It's likely at least one translator will misread the instructions and set it to something other than 'on'. We only want to turn it off if they explicitly say so. This is based on experience with 'rtl' == _x( 'ltr', 'text direction' ).

I imagine there at at least some users who install WordPress in a foreign language but end up blogging in English, which is why I'd want to always load latin.

'no subset' is fine.

@lancewillett
12 years ago

Better off logic, no-subset as default extra subset string

#51 @lancewillett
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [22066]:

Twenty Twelve: better logic for on/off setting to be translated to turn off the font if translators explicitly say so. Also minor fixes to spelling and to use yoda comparison. Fixes #21751.

Note: See TracTickets for help on using tickets.