Skip to content
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(buffer): allow setting falsy values through proxy #11512

Merged
merged 6 commits into from Mar 18, 2020

Conversation

janvennemann
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-27785

setAdjustedIndex was returning the value it did set on the buffer. this value was used as the return value for the proxy's set handler. returning falsy values from there means that the handler failed setting the value which results in error messages like Proxy object's 'set' trap returned falsy value for property '0'

This fixes the handler by always returning true. I also removed the return statement from setAdjustedIndex as it is not used anywhere else.

@build build added this to the 9.1.0 milestone Mar 2, 2020
@build
Copy link
Contributor

build commented Mar 2, 2020

Warnings
⚠️

common/Resources/ti.internal/extensions/node/buffer.js#L76 - common/Resources/ti.internal/extensions/node/buffer.js line 76 – Missing JSDoc parameter description for 'arg'. (valid-jsdoc)

⚠️

common/Resources/ti.internal/extensions/node/buffer.js#L77 - common/Resources/ti.internal/extensions/node/buffer.js line 77 – Missing JSDoc parameter description for 'encodingOrOffset'. (valid-jsdoc)

⚠️

common/Resources/ti.internal/extensions/node/buffer.js#L78 - common/Resources/ti.internal/extensions/node/buffer.js line 78 – Missing JSDoc parameter description for 'length'. (valid-jsdoc)

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 6626 tests are passing.
(There are 699 skipped tests not included in that total)

Generated by 🚫 dangerJS against 4023625

@sgtcoolguy sgtcoolguy self-requested a review March 2, 2020 17:39
@ssaddique
Copy link
Contributor

FR Passed

Test Environment
Studio Ver: 5.1.4.201909061933
SDK Ver: 9.0.0.v20200304064118
OS Ver: 10.14.6
Xcode Ver: Xcode 11.3.1
Appc CLI: 7.1.2
Daemon Ver: 1.1.3
Alloy Ver: 1.14.4
Node Ver: 10.17.0
Emulators: All iOS 13 iPhone simulators.

My testing has found that this bug affects only iOS 13 devices. < iOS 13 there is no error and result displays as expected.
PR ready to be merged, waiting for Jenkins.

@ssaddique ssaddique merged commit a45a8d0 into tidev:master Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants