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

[TIMOB-11709] Android: String.formatTime causes app crash when date i… #9572

Merged
merged 6 commits into from Nov 14, 2017

Conversation

maggieaxway
Copy link
Contributor

@maggieaxway maggieaxway commented Oct 31, 2017

…s string

Update TitaniumModule.java

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

var win = Ti.UI.createWindow({
  backgroundColor: 'white'
});

win.addEventListener('click', function() {
  var date = 'GIBBERISH';
  //var date = new Date();
  alert(String.formatTime(date));
  //alert(String.formatDate(date));
});

win.open();

@build
Copy link
Contributor

build commented Oct 31, 2017

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

Generated by 🚫 dangerJS

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

@maggieaxway, fixing this on the Java side is definitely simpler. I'm just requesting a couple minor changes.

{
int style = DateFormat.SHORT;

return (DateFormat.getTimeInstance(style)).format(time);
if (time != null && time instanceof Date) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to do a null check. It's safe to pass a null reference to instanceof and it'll return false in that case. So, change to this...

if (time instanceof Date) {

return null;
}
}else {
Log.e(TAG, "Error occurred while formatting time please provide a Date()");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this error message from...

Log.e(TAG, "Error occurred while formatting time please provide a Date()");

...to...

Log.e(TAG, "The string.formatTime() function was given an invalid argument. Must be of type 'Date'.");

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Nov 14, 2017

FR Passed.

String.formatTime("String") & String.formatDate("String") throws error :
[ERROR] : [Nexus 6P] TitaniumModule: (main) [2678,2678] The string.formatTime() function was given an invalid argument. Must be of type 'Date'.
[ERROR] : [Nexus 6P] TitaniumModule: (main) [38,2716] The string.formatDate() function was given an invalid argument. Must be of type 'Date'.

No app crash seen.

Studio Ver: 4.10.0.201709271713
SDK Ver: 7.0.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.10
Appc CLI: 6.3.0
Ti CLI Ver: 5.0.14
Alloy Ver: 1.10.7
Node Ver: 7.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Nexus 6P --- Android 8.0.0

@build build added the android label Nov 14, 2017
@eric34 eric34 merged commit f90e8c3 into tidev:master Nov 14, 2017
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

8 participants