New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ios): updated behaviour to original #11354
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, but then I dug deeper...
I think we could fairly easily automate a test for this. Originally my understanding was that a Ti.Blob's toString() method returned it's text value. But I did not consider binary blobs.
So my guess here for the behavior is that if the Blob contains text, toString() should return that, and if it contains binary bytes, it should return "[object TiBlob]"
. In fact, this is exactly what Android's implementation attempts to do.
And in further looking, iOS's TiBlob has a description
method that more or less does the same thing. That appears to be how "old" proxies handled toString() on IOS.
So I think we need to:
- Remove the TiBlob.m
description
method - Add this JS-based fix as you have it
- add some unit tests for TiBlob that test when:
- the blob has empty string content
- the blob has ascii/text content
- the blob has binary content
|
tests/Resources/ti.blob.addontest.js
Outdated
describe('Titanium.Blob', function () { | ||
var win; | ||
|
||
afterEach(function (done) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need for this hook as none of the tests open a window...
should report "[object TiBlob]" Fixes TIMOB-27350
0838a86
to
045ea94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pushed modified commits to have messages match convention and remove the unnecessary test hook. Tests kind of do FR for QE, but should probably double-check with the original ticket's use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FR Passed: Tested using the the test case mentioned in https://jira.appcelerator.org/browse/TIMOB-27350 able to see data received as a blon
[INFO] �� � Received Data: [object TiBlob]
Test Environment
MacOS Catalina 10.15.1 beta
Xcode 11.3
Node.js 10.16.3
"NPM":"4.2.15-1","CLI":"7.1.2-7"
iPhone 8 (ios 13.3)
https://jira.appcelerator.org/browse/TIMOB-27350