WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 22 months ago

#32875 closed defect (bug) (fixed)

Ellipses instead of ... in UI #2

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

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 4 years ago.
hellip_problem.png (8.4 KB) - added by pavelevap 4 years ago.
ticket-32875-u2026.png (17.5 KB) - added by Viper007Bond 4 years ago.
Screenshot showing \u2026 being rendered

Download all attachments as: .zip

Change History (34)

@yoavf
4 years ago

#1 @yoavf
4 years ago

  • Keywords has-patch added; needs-patch removed

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

#2 @jorbin
4 years ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in Slack in #polyglots by zodiac1978. View the logs.


4 years ago

#4 @SergeyBiryukov
4 years ago

  • Milestone changed from Future Release to 4.4

#5 @wonderboymusic
4 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
4 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
4 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
4 years ago

netweb is fired

#9 in reply to: ↑ 8 @netweb
4 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
4 years ago

In 34013:

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

See #32875.

#11 follow-up: @pavelevap
4 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...

#12 @wonderboymusic
4 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
4 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
4 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
4 years ago

How will GlotPress handle \u2026?

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

#16 @wonderboymusic
4 years ago

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

#17 @pavelevap
4 years ago

Which is preferred?

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

#18 @pavelevap
4 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
4 years ago

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

@Viper007Bond
4 years ago

Screenshot showing \u2026 being rendered

#20 in reply to: ↑ 19 @ryan
4 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
4 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
4 years ago

Nevermind, I missed that these are all JS strings.

#23 @pavelevap
4 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
4 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
4 years ago

In 34086:

Themes: Don't use HTML entities for placeholders.

See #32875.

#26 @ocean90
4 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
4 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
4 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.


3 years ago

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


2 years ago

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


22 months ago

Note: See TracTickets for help on using tickets.