Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Refactor to use transform string #428

Merged
merged 2 commits into from Oct 6, 2016

Conversation

yssk22
Copy link
Contributor

@yssk22 yssk22 commented Aug 31, 2016

This PR:

  • improve encoding handling by IA the SDK function transformString and remove the complexities of loadHTML things.

Follows #395

Relates to #369, #344

Fixes #339

Encoding detection/conversion is now being supported from v1.5.0 of Instant Article SDK
by transformString function. We can get rid of 'loadHTML' stuffs from our plugin and
make the code simpler.

This commit also removes the relative url transformation introduced by the comit da90925
(PR #220) but we now should not need this logic on plugin side.
@everton-rosario
Copy link
Collaborator

everton-rosario commented Aug 31, 2016

LGTM
:shipit:

Next version.

@everton-rosario everton-rosario removed this from the 3.1.1 milestone Aug 31, 2016
@LiviuJianu
Copy link

Any chance this gets in the 3.2 milestone?

@everton-rosario everton-rosario added this to the 3.2 milestone Sep 14, 2016
Copy link

@LiviuJianu LiviuJianu left a comment

Choose a reason for hiding this comment

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

Manually made the changes in the plugin already installed in Wordpress and it works ok!

@demoive demoive merged commit 0c6c2da into Automattic:master Oct 6, 2016
demoive added a commit that referenced this pull request Oct 6, 2016
@demoive
Copy link
Contributor

demoive commented Oct 6, 2016

@philipjohn I didn't intend to merge this myself. I simply wanted to resolve the merge conflict, but since it was originally committed straight into master, this was the result. Hope this is ok!

@kawarano
Copy link

Thanks @yssk22 san

@philipjohn some of the merged changes of this PR were reverted with 49c22d0 commit. Could you please re-merge it? It prevents $title which contains multibyte characters from being garbled.

sakatam added a commit to sakatam/facebook-instant-articles-wp that referenced this pull request Feb 24, 2017
- as @yssk22 commented in Automattic#428, the refactored code has  been reverted
  back by mistake, and it result in garbled MB characters in title.
@sakatam sakatam mentioned this pull request Feb 24, 2017
1 task
@sakatam
Copy link
Contributor

sakatam commented Feb 24, 2017

@yssk22 thank you for figuring out! I created a quick PR for this and it'll hopefully be included in the next release.

sakatam added a commit to sakatam/facebook-instant-articles-wp that referenced this pull request Mar 21, 2017
- as @yssk22 commented in Automattic#428, the refactored code has  been reverted
  back by mistake, and it result in garbled MB characters in title.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants