Comment 1 Matt Biggs 2010-05-13 09:48:09 EDT

Additional info:

This only appears to occur when the wizard is cancelled.

Comment 2 Ovidio Mallo 2010-05-26 16:15:04 EDT

Created attachment 170094 [details]
patch fixing the NPE with tests

Matt, the NPE is occuring only when the wizard is canceled since otherwise the uiChanged flag of the DialogPageSupport class is likely to be true in which case the code triggering the NPE is not executed.

Could you maybe check whether this patch solves the problem?

Comment 3 Matt Biggs 2010-05-27 02:50:40 EDT

Ovidio,

Thanks for the patch, it seems to be fixing the problem! Is this likely to be able to make it into 3.6? 

cheers

matt

Comment 4 Ovidio Mallo 2010-05-27 12:43:41 EDT

Matthew, Boris, do you think that this bug could/should be fixed for RC4?

Comment 5 Matthew Hall 2010-05-27 14:09:02 EDT

+1 from me, this looks like a safe change.  Up to Boris on whether it's ok to push this patch at this stage of the release though.

Comment 6 Ovidio Mallo 2010-05-31 15:53:18 EDT

Boris, do you think we could fix this for RC4?

Comment 7 Boris Bokowski 2010-05-31 16:21:14 EDT

Could you attach a version of the patch without formatting changes?

Is this a regression?

Comment 8 Ovidio Mallo 2010-05-31 16:32:58 EDT

Created attachment 170590 [details]
same patch without auto formatting

Comment 9 Ovidio Mallo 2010-05-31 16:34:46 EDT

(In reply to comment #7)
> Could you attach a version of the patch without formatting changes?
Done.

> Is this a regression?
No, it's no regression bug.

Comment 10 Ovidio Mallo 2010-05-31 16:54:10 EDT

What I'm just realising is that with my patch we might not always detach the validationStatusProviderTargetsListener within the DialogPageSupport#dispose() method on the target observable of a Binding (in case the Binding is already disposed) which might lead to problems.

Comment 11 Boris Bokowski 2010-05-31 17:06:54 EDT

I would recommend not fixing in RC4. If we have lived with this for a whole release (probably longer), it rarely qualifies as something that needs fixing just before a release.

Is there a workaround that Matt could be using?

Comment 12 Matt Biggs 2010-06-02 02:43:16 EDT

> Is there a workaround that Matt could be using?

A workaround would be much appreciated. I can't override the class and dispose() method due to the private variables used within the DialogPageSupport.

As it stands, with this issue, i have to cancel all wizards twice to remove them.

Comment 13 Boris Bokowski 2010-06-02 11:06:47 EDT

Could you make your own copy of WizardPageSupport/DialogPageSupport with a fix? After all, it's just a helper class.

Comment 14 Matt Biggs 2010-06-02 11:09:52 EDT

(In reply to comment #13)
> Could you make your own copy of WizardPageSupport/DialogPageSupport with a fix?
> After all, it's just a helper class.

I did think that may be an option. Its only 2 classes. Would this fix make it into 3.6 SR1 then?

Comment 15 Ovidio Mallo 2010-06-02 13:20:53 EDT

Sorry for the delay, Matt. Another option might also be to make sure that the ValidationStatusProvider#getTargets() method does not return null after disposal. The only ValidationStatusProvider of the data binding framework which does this is MultiValidator so you might override MultiValidator (you must extend it anyway) and return an empty list instead of null in case the MultiValidator is disposed (see MultiValidator#isDisposed()).

Comment 16 Boris Bokowski 2010-06-02 14:00:51 EDT

(In reply to comment #14)
> (In reply to comment #13)
> > Could you make your own copy of WizardPageSupport/DialogPageSupport with a fix?
> > After all, it's just a helper class.
> 
> I did think that may be an option. Its only 2 classes. Would this fix make it
> into 3.6 SR1 then?

Yes, fixing for 3.6.1 shouldn't be a problem.

Comment 17 Gemma Bowers 2010-06-02 16:06:47 EDT

Thanks for the updates. 

Overriding the MultiValidator sounds interesting. ill have a look at that. 

Fixing for 3.6.1 would be much appreciated! Thanks.

Comment 18 Matt Biggs 2010-06-02 16:13:46 EDT

(In reply to comment #17)
> Thanks for the updates. 
> 
> Overriding the MultiValidator sounds interesting. ill have a look at that. 
> 
> Fixing for 3.6.1 would be much appreciated! Thanks.

Interesting, i wrote that above comment but it appears my partner was logged in! oops.

Matt

Comment 19 Matt Biggs 2010-06-03 04:10:14 EDT

Adding the following to the MultiValidator stops the NPE occuring. Thanks

@Override
public IObservableList getTargets() {
	/**
	 * BUGFIX: https://bugs.eclipse.org/bugs/show_bug.cgi?id=312785
	 */
	if( isDisposed() ) {
		return new ObservableList(Collections.emptyList(), null) {
		};
	}
	return super.getTargets();
}

Comment 20 Ovidio Mallo 2010-06-03 12:34:56 EDT

(In reply to comment #19)
> Adding the following to the MultiValidator stops the NPE occuring. Thanks
I hope this temporary fix is OK for you for the time being.

BTW, you might also use Observables#emptyObservableList().

Comment 21 Matt Biggs 2010-06-03 13:06:22 EDT

> BTW, you might also use Observables#emptyObservableList().


aha... i was looking for something like that, but couldn't find it. Thanks for the tip!

Comment 22 Matthew Hall 2011-09-20 00:55:06 EDT

Ovidio, it looks like we're pretty close on this one, except we need a unit test for the TitleAreaDialog part. Can we push this through for M3?

Comment 23 Brian de Alwis 2016-08-25 10:44:21 EDT

Ovido — I'd like to apply this patch.  Would it be possible for you to sign the Eclipse Contributors Agreement (https://wiki.eclipse.org/ECA)?

Comment 24 Eclipse Genie 2020-05-25 17:27:19 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.