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-25431] Improve getActivity() validation #10017

Merged
merged 8 commits into from May 23, 2018

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Apr 30, 2018

  • getActivity() must always return a valid activity. There are many instances where we assume getActivity() to return a valid activity TiUILabel#L71, TiTableView.java#L307 etc...
  • Add validation to TiUICardView.getLayout()

JIRA Ticket

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

CR looks good to me.

@garymathews garymathews force-pushed the TIMOB-25431 branch 2 times, most recently from 7f784b2 to 97ff780 Compare May 1, 2018 18:43
@build build added the android label May 1, 2018
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

@drauggres
Copy link
Contributor

getActivity() must always return a valid activity.

Looks like bad idea to me.

There are many instances where we assume getActivity() to return a valid activity

Probably we should check getActivity() result for null and for .isDestroyed() (smth like #9579)

@jquick-axway
Copy link
Contributor

@drauggres, while I agree with you, the problem is too much code (especially modules) assumes this method never returns null and the stack-trace makes it look like a core problem on our end. This issue is already too wide spread. So, with this change, this method will grab the 1st launched splash-screen activity if no other child activity exists on the UI stack since it's guaranteed to exist.

@build build added the android label May 8, 2018
@build
Copy link
Contributor

build commented May 8, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@sgtcoolguy sgtcoolguy removed this from the 7.2.0 milestone May 16, 2018
@sgtcoolguy sgtcoolguy added this to the 7.3.0 milestone May 16, 2018
@sgtcoolguy sgtcoolguy merged commit cb37fca into tidev:master May 23, 2018
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

5 participants