Skip to content

Conversation

@matthiasblaesing
Copy link
Contributor

@matthiasblaesing matthiasblaesing commented Sep 6, 2018

…s.addPropertyChangeListener.

WeakListeners.propertyChange(lister, source) expected, that it can
detach from the supplied source by invoking removePropertyChangeListener
on it. This contract is violated in this construct:

DocumentUtilities.addPropertyChangeListener(doc, WeakListeners.propertyChange(this, doc));

DocumentUtilities does not attach the PropertyChangeListener to the
document, but to a PropertyChangeLister contained in it. The Document
itself does not support removePropertyChangeListener.

The solution is to add a new api method addWeakPropertyChangeListener,
that hides the complexity from the user.

The issue was raised and diagnosed by Eirik Bakke.

Closes: #515

@matthiasblaesing
Copy link
Contributor Author

This changeset is based on discussion in PR #515 - the comments there might help with review. As I suggested the fix, I decided to finally take it on and get it done.

@eirikbakke
Copy link
Contributor

Brilliant--thank you!

is.autoload=true
javac.source=1.7
javac.compilerargs=-Xlint:unchecked
spec.version.base=2.20.0
Copy link
Member

Choose a reason for hiding this comment

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

Please increment minor version; micro one is incremented for branched release patches AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I updated the commit accordingly. I'll wait for travis to do its job and push tonight, thanks for checking.

…s.addPropertyChangeListener.

WeakListeners.propertyChange(lister, source) expected, that it can
detach from the supplied source by invoking removePropertyChangeListener
on it. This contract is violated in this construct:

DocumentUtilities.addPropertyChangeListener(doc, WeakListeners.propertyChange(this, doc));

DocumentUtilities does not attach the PropertyChangeListener to the
document, but to a PropertyChangeLister contained in it. The Document
itself does not support removePropertyChangeListener.

The solution is to add a new api method addWeakPropertyChangeListener,
that hides the complexity from the user.

The issue was raised and diagnosed by Eirik Bakke.
@matthiasblaesing matthiasblaesing merged commit cc5a743 into apache:master Sep 7, 2018
@matthiasblaesing matthiasblaesing deleted the netbeans-406 branch October 6, 2018 07:48
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.

3 participants