You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Noel Grandin <no...@gmail.com> on 2012/05/15 14:10:32 UTC
Re: svn commit: r1338663 - in /pivot/trunk/wtk/src/org/apache/pivot/wtk:
TreeView.java content/TableViewMultiCellRenderer.java skin/TextPaneSkinVerticalElementView.java
Note that Eclipse's null checker is wrong in these cases, because the if
conditions mean that it can never be null in those places.
The checker is pretty smart, but it's not always right.
On 2012-05-15 14:07, smartini@apache.org wrote:
> Author: smartini
> Date: Tue May 15 12:07:39 2012
> New Revision: 1338663
>
> URL: http://svn.apache.org/viewvc?rev=1338663&view=rev
> Log:
> fix potential NPE
>
> Modified:
> pivot/trunk/wtk/src/org/apache/pivot/wtk/TreeView.java
> pivot/trunk/wtk/src/org/apache/pivot/wtk/content/TableViewMultiCellRenderer.java
> pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TextPaneSkinVerticalElementView.java
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/TreeView.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/TreeView.java?rev=1338663&r1=1338662&r2=1338663&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/TreeView.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/TreeView.java Tue May 15 12:07:39 2012
> @@ -1640,7 +1640,7 @@ public class TreeView extends Component
> Path ancestorPath = new Path(path, path.getLength() - 1);
>
> for (int i = ancestorPath.getLength() - 1; i>= 0; i--) {
> - NodeCheckState ancestorPreviousCheckState = ancestorCheckStates.get(i);
> + NodeCheckState ancestorPreviousCheckState = (ancestorCheckStates != null) ? ancestorCheckStates.get(i) : null;
> NodeCheckState ancestorCheckState = getNodeCheckState(ancestorPath);
>
> if (ancestorCheckState != ancestorPreviousCheckState) {
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/content/TableViewMultiCellRenderer.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/content/TableViewMultiCellRenderer.java?rev=1338663&r1=1338662&r2=1338663&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/content/TableViewMultiCellRenderer.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/content/TableViewMultiCellRenderer.java Tue May 15 12:07:39 2012
> @@ -275,20 +275,20 @@ public class TableViewMultiCellRenderer
> }
>
> @Override
> - public int getPreferredWidth(int height) {
> - return currentRenderer.getPreferredWidth(height);
> + public int getPreferredWidth(int heightArgument) {
> + return currentRenderer.getPreferredWidth(heightArgument);
> }
>
> @Override
> - public int getPreferredHeight(int width) {
> + public int getPreferredHeight(int widthArgument) {
> // Our preferred height is the maximum of all our possible renderers'
> // preferred height
> - int preferredHeight = defaultRenderer.getPreferredHeight(width);
> + int preferredHeight = defaultRenderer.getPreferredHeight(widthArgument);
>
> for (Class<?> key : cellRenderers) {
> TableView.CellRenderer renderer = cellRenderers.get(key);
> preferredHeight = Math.max(preferredHeight,
> - renderer.getPreferredHeight(width));
> + renderer.getPreferredHeight(widthArgument));
> }
>
> return preferredHeight;
> @@ -300,7 +300,7 @@ public class TableViewMultiCellRenderer
> }
>
> @Override
> - public int getBaseline(int width, int height) {
> + public int getBaseline(int widthArgument, int heightArgument) {
> return -1;
> }
>
> @@ -329,7 +329,7 @@ public class TableViewMultiCellRenderer
> cellRenderer = cellRenderers.get(valueClass);
>
> if (cellRenderer == null) {
> - valueClass = valueClass.getSuperclass();
> + valueClass = (valueClass != null) ? valueClass.getSuperclass() : null;
> }
> }
>
> @@ -358,7 +358,7 @@ public class TableViewMultiCellRenderer
> cellRenderer = cellRenderers.get(valueClass);
>
> if (cellRenderer == null) {
> - valueClass = valueClass.getSuperclass();
> + valueClass = (valueClass != null) ? valueClass.getSuperclass() : null;
> }
> }
>
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TextPaneSkinVerticalElementView.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TextPaneSkinVerticalElementView.java?rev=1338663&r1=1338662&r2=1338663&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TextPaneSkinVerticalElementView.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TextPaneSkinVerticalElementView.java Tue May 15 12:07:39 2012
> @@ -151,7 +151,7 @@ abstract class TextPaneSkinVerticalEleme
> }
>
> if (offset != -1) {
> - offset += nodeView.getOffset();
> + offset += (nodeView != null) ? nodeView.getOffset() : 0;
> }
> }
> }
>
>
Re: svn commit: r1338663 - in /pivot/trunk/wtk/src/org/apache/pivot/wtk:
TreeView.java content/TableViewMultiCellRenderer.java skin/TextPaneSkinVerticalElementView.java
Posted by Noel Grandin <no...@gmail.com>.
Look at the whole method, not just the few lines in a diff.
For example, in TreeView, the ancestorCheckStates variable is accessed
like this:
Sequence<NodeCheckState> ancestorCheckStates = null;
if (showMixedCheckmarkState) {
ancestorCheckStates = new
ArrayList<NodeCheckState>(path.getLength() - 1);
......
}
.......
if (showMixedCheckmarkState) {
.....
for (int i = ancestorPath.getLength() - 1; i >= 0; i--) {
NodeCheckState ancestorPreviousCheckState =
ancestorCheckStates.get(i);
....
}
}
So in the places where it is accessed, it can never be null.
The same applies to the other 2 changes in this diff.
On 2012-05-15 15:19, Sandro Martini wrote:
> Hi Noel,
>
>> Note that Eclipse's null checker is wrong in these cases, because the if conditions mean that it can never be null in those places.
>>
>> The checker is pretty smart, but it's not always right.
> Are you sure ? Looking at that code it seems that (in some cases)
> could happen to have related variables null, so my additional check
> ... but note that looking only at diff it's not so clear ...
>
> http://svn.apache.org/viewvc?view=revision&revision=1338663
>
>
> If there aren't objections I'd try to keep these changes for some day,
> and in the meantime I'm testing/retisting everything many times and in
> many ways :-) .
>
>
> Bye
Re: svn commit: r1338663 - in /pivot/trunk/wtk/src/org/apache/pivot/wtk:
TreeView.java content/TableViewMultiCellRenderer.java skin/TextPaneSkinVerticalElementView.java
Posted by Sandro Martini <sa...@gmail.com>.
Hi Noel,
> Note that Eclipse's null checker is wrong in these cases, because the if conditions mean that it can never be null in those places.
>
> The checker is pretty smart, but it's not always right.
Are you sure ? Looking at that code it seems that (in some cases)
could happen to have related variables null, so my additional check
... but note that looking only at diff it's not so clear ...
http://svn.apache.org/viewvc?view=revision&revision=1338663
If there aren't objections I'd try to keep these changes for some day,
and in the meantime I'm testing/retisting everything many times and in
many ways :-) .
Bye