Don't break encoding of non-ISO-8859-1 data #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wiese
reviewed
May 24, 2018
src/Component.php
Outdated
| $entityLoaderDisabled = libxml_disable_entity_loader( true ); | ||
| $internalErrors = libxml_use_internal_errors( true ); | ||
| $document = new DOMDocument(); | ||
| $document = new DOMDocument( null, 'UTF-8' ); |
There was a problem hiding this comment.
Probalbly version "1.0" is the righter thing™ to do, albeit the outcome will be the same.
| ] | ||
| } | ||
| </script> | ||
| <div id="result"> | ||
| <!-- generated by `npm run-script populate-fixtures` --> | ||
| <div class="lemma-widget"><ul class="lemma-widget_lemma-list"><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">lemma1</span> <span class="lemma-widget_lemma-language">en</span></li><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">lemma2</span> <span class="lemma-widget_lemma-language">fr</span></li></ul> <div class="lemma-widget_controls"><button type="button" class="lemma-widget_control">wikibase-edit</button> <!----> <!----></div></div> | ||
| <div class="lemma-widget"><ul class="lemma-widget_lemma-list"><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">lemma1</span> <span class="lemma-widget_lemma-language">en</span></li><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">lémma2</span> <span class="lemma-widget_lemma-language">fr</span></li><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">기본형</span> <span class="lemma-widget_lemma-language">ko</span></li><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">Лемма</span> <span class="lemma-widget_lemma-language">ru</span></li><li class="lemma-widget_lemma"><span class="lemma-widget_lemma-value">لليما</span> <span class="lemma-widget_lemma-language">ar</span></li></ul> <div class="lemma-widget_controls"><button type="button" class="lemma-widget_control">wikibase-edit</button> <!----> <!----></div></div> |
There was a problem hiding this comment.
So this is really the outcome of npm run-script populate-fixtures - and it contains html entities for characters that, in an UTF-8 world could be expressed without html entities. In the clients we do not want this to happen (right?!) so why do we have a test proving the opposite?
| [ 'value' => '<p>한국어</p>' ] | ||
| ); | ||
|
|
||
| assertThat( $result, is( equalTo( '<div><div><p>한국어</p></div></div>' ) ) ); |
There was a problem hiding this comment.
Not quite to the point of this bug, but we talked about is earlier... if v-html="value" is not part of the rendered mark-up there is no way for client-side JS code to find the respective node for data binding. So what is really the benefit of having both mustache and the v-html way of interpolating variables?
As Vue rendering does not do it either. Makes for more clear expectations from the PHP renderer.
wiese
approved these changes
May 24, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://phabricator.wikimedia.org/T195359