Closed simonw closed 8 years ago
This fails the tests because it increases the size of any payloads that use non-ASCII characters (dramatically - you're trebling the number of bytes used to represent two-byte UTF-8 characters, and doubling the number needed to represent three-byte UTF-8 characters, since JSON's ASCII-only escape sequences for non-ASCII characters are much less byte-efficient than UTF-8).
I don't think this is a tradeoff we can accept. If we merged this, we'd be significantly reducing the maximum message length that could be sent in non-Latin-alphabet languages like Chinese / Arabic / Hebrew, and we could cause errors or significant increases in required bandwidth for people updating their PyAPNs version on what was previously a stable working deployment.
Using JSON escape sequences for emojis instead of UTF-8 encoding them may be the right fix here (I don't have a clue why the bug described in https://github.com/djacobs/PyAPNs/issues/127 exists in the first place, so I'm poorly qualified to suggest solutions), but this PR is evil in its current state. Closing (though hopeful that you'll figure out a less problematic solution and open a new PR!)
Refs #127 - this change allows unicode Emoji sequences to be sent as push notification banners, e.g.: