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

JBIDE-19410: Avoid pop-up (show error on UI and in log instead) #328

Closed
wants to merge 1 commit into from
Closed

JBIDE-19410: Avoid pop-up (show error on UI and in log instead) #328

wants to merge 1 commit into from

Conversation

mickaelistria
Copy link
Contributor

No description provided.

HeaderText.this.hasEarlyAccessIUs = InstallationChecker.getInstance().hasEarlyAccess();
public IStatus run(IProgressMonitor monitor) {
try {
HeaderText.this.installChecker = InstallationChecker.getInstance();
Copy link
Member

Choose a reason for hiding this comment

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

you prolly want to check HeaderText is not disposed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line doesn't do anything related to UI, so there is no point in checking whether it's disposed or not.

Copy link
Member

Choose a reason for hiding this comment

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

UIJob here is required, because updateTitle(heading) requires display.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I don't get how I could miss this issue...
See amended commit. However, I prefer running as few things as possible in UIThread since it creates UI freezes and slowness. I prefered encapsulating the UI changes in a Display.exec().

@mickaelistria
Copy link
Contributor Author

@fbricon What's still blocking to merge that?

} catch (ProvisionException ex) {
JBossCentralActivator.log(ex);
}
heading.getDisplay().syncExec(new Runnable() {
Copy link
Member

Choose a reason for hiding this comment

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

heading.getDisplay() will failed if disposed

@mickaelistria
Copy link
Contributor Author

Change was amended to fix the last issue you've spotted.

@fbricon
Copy link
Member

fbricon commented Apr 14, 2015

applied in master

@fbricon fbricon closed this Apr 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants