Fix: open as text link on downloads now uses utf-8 encoding #1109
Conversation
| // FIXME: is this safe? See bug #131 | ||
| MediaType textMediaType = new MediaType("text/plain"); | ||
| MediaType textMediaType = new MediaType("text/plain;charset=utf-8"); | ||
| textMediaType.setParameter("charset", (e.getExpectedMimeType() != null) ? MediaType.getCharsetRobust(e.getExpectedMimeType()) : null); |
There was a problem hiding this comment.
It kinda looks like this line would immediately clobber whatever you’ve set in the constructor…
There was a problem hiding this comment.
damn, sometimes I’m dumb and blind … thank you for catching it. This would have been so much easier if I hadn’t done that late at night and left it uncommitted for weeks …
There was a problem hiding this comment.
No, this would have been easier if you had written a test first. 😄
(I’m currently trying to write one, but this would be easier if somebody merged #1043 first, because that one contains a lot of shit for easier tests. 🙂)
There was a problem hiding this comment.
Do you really mean the "quote n2ntm PR"?
(it feels like the quoting PR still needs so much work that I should only finish it after the next release)
There was a problem hiding this comment.
Actually, both #1043 and #1106, I need the added dependencies for this test I’m currently building for this… but I’d actually settle for #1106, it’s more important, we really need the updated Mockito version. 🙂
…but I can already say: nope, this also doesn’t work. 😁 (Actually, the next line [953] doesn’t.)
Thanks to Bombe for the review!
2 participants
Tiny change, replaces most of the use-cases of the much more complex and incorrect #970