Problem/Motivation
See #2566503: [meta] Replace remaining !placeholder for Non-URL HTML outputs only
format_size()
is can be inserted into translated strings using !placeholder
.
Note we also need to check for objects that wrap it like MigrateExecutable...
/**
* Wraps format_size()
*
* @return string
* The formatted size.
*/
protected function formatSize($size) {
return format_size($size);
}
Proposed resolution
It is totally safe to replace ! with @ here. Since format_size() is supposed to return output from t(). However it's implementation completely breaks the safe list. That is being fixed in #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list since after that it will return a TranslationWrapper.
There are many places in core that already use an @placeholder
or %placeholder
. For example:
drupal_set_message(t('There is not enough memory available to PHP to change this theme\'s color scheme. You need at least %size more. Check the <a href="@url">PHP documentation</a> for more information.', array('%size' => format_size($usage + $required - $size), '@url' => 'http://www.php.net/manual/ini.core.php#ini.sect.resource-limits')), 'error');
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff.txt | 2.15 KB | joelpittet |
#8 | replace_remaining-2568793-8.patch | 3.25 KB | joelpittet |
#5 | 2568793-5.patch | 3.1 KB | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
alexpottComment #5
alexpottGiven that the
@placeholder
and%placeholder
usage with respect toformat_size()
far far outweighs!placeholder
usage I think it is not worth adding any test coverage here. A simple review with--color-words
should suffice.Comment #6
dawehnerYeah this looks alright
Comment #7
alexpottThis is part of the critical meta.
Comment #8
joelpittetConverted this !pct value here too as it's going to likely cause conflicts anyways we look at it and the scope impact is minimal. Also these are the only instances of
!pct
in all of core.Comment #9
klausiLooks good, verified with git diff --color-words that only the placeholder character was changed.
Comment #10
star-szrI manually tested the first hunk. It checks out!
Comment #11
star-szrMarkup before and after is identical (Twig debug comments included for context):
Before:
After:
Comment #12
xjmThanks @Cottser!
Committed and pushed to 8.0.x.