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

Prevent broken translations in some locales for the edit post link #1066

Merged
merged 3 commits into from
Jun 9, 2017

Conversation

yoavf
Copy link
Contributor

@yoavf yoavf commented Jan 3, 2017

Since #775, the translation of the edit post link in some languages can end up forming a partial sentence.

By default, in English, the visual output is the word "Edit".
In Hebrew for example, the translation would be a single word:

לערוך

With the addition of the post title, the translator is instructed to translate Edit %s with %s being the post name. In Hebrew, the translation will be

לערוך את %s

Note the added word את

To quote from this post:

The word את comes before a definite object. It has no equivalent in the English language.

The end visual result in Hebrew would be:

לערוך את

Which is weird. Think of it as "Edit The" (without the actual object).

This PR fixes that by expanding the comment to note that the post name only visible to screen readers, and puts the screen reader span html inside the translated string.
The translator for Hebrew can now do something like
לערוך <span class="screen-reader-text">את %s</span>, putting the את word inside the invisible span.

@yoavf yoavf added the Has Patch label Jan 3, 2017
Copy link
Collaborator

@grappler grappler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise 👍

esc_html__( 'Edit %s', '_s' ),
the_title( '<span class="screen-reader-text">"', '"</span>', false )
/* translators: %s: Name of current post. Only visible to screen readers */
__( 'Edit <span class="screen-reader-text">%s</span>', '_s' ),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the function was esc_html__(). It is best not to change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's won't work now that we have the html back in the string.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, then we will need to use wp_kses() like done here: https://github.com/Automattic/_s/blob/master/inc/template-tags.php#L65

@yoavf yoavf changed the title Fix issue Prevent broken translations in some locales for the edit post link Jan 3, 2017
Copy link
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this could have been fixed by just adjusting the translators comment to be more clear, rather than changing the actual code.

@yoavf
Copy link
Contributor Author

yoavf commented Jan 3, 2017

believe this could have been fixed by just adjusting the translators comment to be more clear, rather than changing the actual code.

My first instinct was to do just that - but that means (for Hebrew) choosing to omit a word from the screen reader version. It is not a better outcome.

@davidakennedy
Copy link
Contributor

Looks good! Thanks @yoavf @grappler and @jrfnl!

@davidakennedy davidakennedy merged commit 3aa7934 into Automattic:master Jun 9, 2017
davidakennedy added a commit to davidakennedy/_s that referenced this pull request Jun 9, 2017
davidakennedy added a commit to davidakennedy/_s that referenced this pull request Jun 9, 2017
davidakennedy added a commit to davidakennedy/_s that referenced this pull request Jun 9, 2017
Tweak Automattic#1066 to fix build errors

Adjust location of placeholder text
davidakennedy added a commit that referenced this pull request Jun 9, 2017
gatespace added a commit to gatespace/_s that referenced this pull request Jun 15, 2017
* upstream/master: (31 commits)
  Improve "Continue reading" word order for i18n
  Create readme.txt
  Remove invalid rel="designer" attribute
  Fix build errors related to Automattic#1066
  Adjust location of placeholder text
  Tweak Automattic#1066 to fix build errors
  Fix build errors related to Automattic#1066
  Remove tags
  Update README.md to be consistent with new file name
  Replace extras.php with template-functions.php
  Update WordPress version and copyright date
  Switch versions of PHP Code Sniffer for build process
  Adding ID to comment labels for post
  CS: Fix code layout of nested function calls with associative arrays.
  I18n: ensure all text strings with placeholders have translators comments.
  Code obfuscation: Fix assignment within a condition
  I18n: Fix a translation call for a text string which needs context
  Update readme.txt
  doctype uppercase to lowercase
  clarify who maintains _s and standardize returns in CONTRIBUTING.md
  ...

# Conflicts:
#	languages/_s.pot
gatespace added a commit to gatespace/_s that referenced this pull request Jun 15, 2017
* Update copyright year.
* Add comments for translators.
* Translation fix for a comment strings
* Added translation word by code change. see Automattic#1100, Automattic#1066 Automattic#1117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants