WordPress.org
Get WordPress

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#32875 closed defect (bug) (fixed)

Ellipses instead of ... in UI #2

Reported by: netweb's profile netweb Owned by: ocean90's profile ocean90
Milestone: 4.4 Priority: normal
Severity: minor Version: 4.3
Component: Text Changes Keywords: has-patch
Focuses: Cc:

Description

We should use ellipses … / … instead of three dots/periods ... e.g Loading… not Loading...

This was originally updated in #8714 though since then a few have snuck into the codebase

Getting started with grep: grep -in "\.\.\." src/ -r

This ​patch isn't 100% accurate based on feedback from @Ocean90 ​here on Slack:

Looks like … works in placeholders too, but not when using $.text(), see ​http://jsfiddle.net/u241cuy1/1/

Attachments (3)

32875.diff​ (7.5 KB) - added by yoavf 11 years ago.
hellip_problem.png​ (8.4 KB) - added by pavelevap 11 years ago.
ticket-32875-u2026.png​ (17.5 KB) - added by Viper007Bond 11 years ago.
Screenshot showing \u2026 being rendered

Download all attachments as: .zip

Change History (34)

@yoavf
11 years ago

#1 @yoavf
11 years ago

  • Keywords has-patch added; needs-patch removed

32875.diff​ uses the … char for strings used in JS.

#2 @jorbin
11 years ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in ​Slack in #polyglots by zodiac1978. ​View the logs.
11 years ago

#4 @SergeyBiryukov
11 years ago

  • Milestone changed from Future Release to 4.4

#5 @wonderboymusic
11 years ago

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

In 33970:

Round 2 of: We should use ellipses … / … instead of three dots/periods ... e.g Loading… not Loading...

Props yoavf.
Fixes #32875.

#6 @westonruter
11 years ago

I'm seeing here literal Unicode characters encoded as UTF-8 being committed to the source:

'saving' => __( 'Saving…' ),

When I saw this I was concerned for when another encoding is used for a blog (e.g. Latin1).

Given this test case:

<html>
<meta charset="latin1">
<?php
$l10n = array(
        'updating' => __( 'Updating…' ),
);
?>
<p><?php echo $l10n['updating']; ?></p>
<script>
        var l10n = <?php echo wp_json_encode( $l10n ) ?>;
        document.write( l10n.updating );
</script>
</html>

When viewed in my browser, I see the PHP-rendered text being corrupted, whereas the text which was encoded to JSON and written out by JS, succeeded:

Updating…

Updating…

This is because json_encode() does the right thing (by default) to encode characters as Unicode escape sequences, so in the the example above, an ellipsis gets encoded as \u2026:

var l10n = {"updating":"Updating\u2026"};

So that is why it works. However if wp_json_encode() gets called with the JSON_UNESCAPED_UNICODE flag, then this JS example will also come out as mojibake. It looks like the patch is accounting for this by only using UTF-8 encoded characters in strings that will be serialized to JSON, but it's something to be mindful of.

Also, committers will have to be careful to make sure that their editors are configured to use the UTF-8 text encoding, (which should be the default now anyway), or else accidentally re-encode characters in UTF-8 into another encoding.

#7 @pento
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I thought we had a rule (or at least a habit) of not committing non-ASCII characters, for this reason. It's okay to assume that core committers have their editor encoding set to UTF-8, but we can't really assume that everyone does.

I'm inclined to swap the UTF-8 characters for their \u2026 escape sequence, instead, as @westonruter noted.

#8 follow-up: @wonderboymusic
11 years ago

netweb is fired

#9 in reply to: ↑ 8 @netweb
11 years ago

Replying to wonderboymusic:

netweb is fired

Bwahahaha... Wait a min... I didn'... It was... Meh, I'll take the #blame for the greater good ;P

#10 follow-up: @wonderboymusic
11 years ago

In 34013:

After [33970], swap UTF-8 characters for their \u2026 escape sequence.

See #32875.

#11 follow-up: @pavelevap
11 years ago

Do we need both strings?

Saving\u2026

Saving&hellip;

I am not sure that it is the best solution for translators...

Also see attached screenshot, does not seem good...

@pavelevap
11 years ago

#12 @wonderboymusic
11 years ago

In 34014:

_wpThemeSettings.l10n.searchPlaceholder is an input placeholder and doesn't handle UTF-8 or HTML entities properly. In lieu of using literal Unicode here, use ....

See #32875.

#13 in reply to: ↑ 10 ; follow-up: @ocean90
11 years ago

  • Keywords good-first-bug removed

Replying to wonderboymusic:

In 34013:

After [33970], swap UTF-8 characters for their \u2026 escape sequence.

See #32875.

Ugh, not sure if this needs some explanation for translators.

#14 in reply to: ↑ 11 @wonderboymusic
11 years ago

Replying to pavelevap:

Do we need both strings?

Saving\u2026
Saving&hellip;

I am not sure that it is the best solution for translators...

Which is preferred?

#15 @pavelevap
11 years ago

How will GlotPress handle \u2026?

And one more hellip input placeholder problem with "Search themes" string...

#16 @wonderboymusic
11 years ago

  • Owner changed from wonderboymusic to ocean90
  • Status changed from reopened to assigned

#17 @pavelevap
11 years ago

Which is preferred?

I have no preference, but seems unnecessarily to me to have both.

#18 @pavelevap
11 years ago

Now there will be also other strings:

Search installed themes&hellip; (imput placeholder)

Search installed themes... (screen reader text)

#19 follow-up: @Viper007Bond
11 years ago

I'm seeing \u2026 in some places now. See incoming screenshot.

@Viper007Bond
11 years ago

Screenshot showing \u2026 being rendered

#20 in reply to: ↑ 19 @ryan
11 years ago

Replying to Viper007Bond:

I'm seeing \u2026 in some places now. See incoming screenshot.

I see the same for every plugin update.

#21 in reply to: ↑ 13 @SergeyBiryukov
11 years ago

Replying to ocean90:

Replying to wonderboymusic:

In 34013:

After [33970], swap UTF-8 characters for their \u2026 escape sequence.

See #32875.

Ugh, not sure if this needs some explanation for translators.

Let's please use &hellip; where we can instead of \u2026, for consistency with the rest of translatable strings.

#22 @SergeyBiryukov
11 years ago

Nevermind, I missed that these are all JS strings.

#23 @pavelevap
11 years ago

What about adding placeholder with translators comment? Something like Search installed themes%s? We have also special &hellip; string which can be translated so strings could be also without placeholder (I am not sure about RTL consequences) and &hellip; or … outside of translatable strings?

#24 @ocean90
11 years ago

We are seeing \u2026 in some places because it gets escaped by json_encode():

$a = [];
$a['save'] = 'Saving\u2026';
$a['save2'] = 'Saving…';
var_dump( json_encode( $a ) ); // string(47) "{"save":"Saving\\u2026","save2":"Saving\u2026"}"

#25 @ocean90
11 years ago

In 34086:

Themes: Don't use HTML entities for placeholders.

See #32875.

#26 @ocean90
11 years ago

In 34087:

Revert [34013] and parts of [33970].

  • _WP_Editors::wp_mce_translation() can't be changed without changing strings in TinyMCE and plugins.
  • \u2026 is escaped by json_encode() to \\u2026, makes \u2026 visible in our UI.

See #32875.

#27 @afercia
11 years ago

How about to add the same inline comment (it was there for a good reason I guess) to all the reverted lines? :)
// (no ellipsis)

#28 @SergeyBiryukov
11 years ago

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

In 34233:

Add a comment to strings where the ellipsis cannot be used due to json_encode(), placeholders, or external dependencies.

Fixes #32875.

This ticket was mentioned in ​Slack in #core by netweb. ​View the logs.
10 years ago

This ticket was mentioned in ​Slack in #core-editor by netweb. ​View the logs.
9 years ago

This ticket was mentioned in ​Slack in #core-i18n by audrasjb. ​View the logs.
8 years ago

Note: See TracTickets for help on using tickets.