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

Yet another RootPanel NPE fix. And general bonus-cleanup. #121

Merged
merged 5 commits into from
Mar 12, 2017

Conversation

huxi
Copy link
Contributor

@huxi huxi commented Mar 11, 2017

Unfortunately, another NPE lurked in getScrollableTracksViewportHeight().

I didn't immediately spot it because those happen in addNotify/removeNotify context and are thus easy to miss.

I have taken the opportunity to sanity-check every single usage of enclosingScrollPane and identified some more places that didn't perform necessary null checks. Fixed them.

Since FS requires Java 6 anyway, I also streamlined handling of documentListeners in 9078cf8, preventing more potential NPEs (according to documentation) and cleaning up the code in the process.

Access to documentListeners and enclosingScrollPane has been locked down. This introduces potential compile-time compatibility issues for people extending RootPanel/BasicPanel but changes necessary to fix them are very straightforward.

@pbrant My previous PR aimed to be minimal. This one aims to be thorough. Sorry for the hassle.

huxi added 5 commits March 11, 2017 10:46
- documentListeners is now Set<DocumentListener> instead of Map
- added protected hasDocumentListeners() method
- using foreach for documentListeners iteration
- moved addDocumentListener and removeDocumentListener from BasicPanel to RootPanel
- fixed null handling of addDocumentListener and removeDocumentListener according to javadoc
- documentListeners is now private and final
- enclosingScrollPane is now private
- added protected getEnclosingScrollPane()
- removed BasicPanel.getFixedRectangle() because it was identical to RootPanel implementation.

This ensures that initialization performed in setEnclosingScrollPane can’t be circumvented.
@pbrant
Copy link
Member

pbrant commented Mar 12, 2017

Thanks!

@pbrant pbrant merged commit 75ef068 into flyingsaucerproject:master Mar 12, 2017
@huxi huxi deleted the YetAnotherRootPanelNPE branch March 12, 2017 18:09
@pbrant
Copy link
Member

pbrant commented Mar 13, 2017

I've released this as https://github.com/flyingsaucerproject/flyingsaucer/releases/tag/v9.1.4 yesterday. It's available on Central.

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.

2 participants