Wednesday, March 11, 2009

0.2 Release

Well, I believe I'm ready to release 0.2. If you read my last blog, I was currently evaluating what changes I need to make between two files (AddExtensionsComponentDialog.java, AbstractExtensionsSection.java). I figured out that AbstractExtensionsSection.java was the file I was looking for.

I ran a find for "removeButton.setEnabled" (the method which enables and disables buttons) within this file and I found two lines, one of which was located within a method called refresh(). The name alone suggested that this might be relevant. It would make sense for the section to be refreshed on a consistent basis. Here is the code:


public void refresh()
{
setListenerEnabled(false);
if (input != null)
{
if ( prevInput == input)
return;
else
prevInput = input;

Tree tree = extensionTreeViewer.getTree();
extensionDetailsViewer.setInput(null);
tree.removeAll();

addButton.setEnabled(!isReadOnly);

extensionTreeViewer.setInput(input);
if (tree.getSelectionCount() == 0 && tree.getItemCount() > 0)
{
TreeItem treeItem = tree.getItem(0);
extensionTreeViewer.setSelection(new StructuredSelection(treeItem.getData()));
}

removeButton.setEnabled(tree.getSelectionCount() > 0 && !isReadOnly);

// Bugzilla 197315. Make this bulletproof for maintenance release.
Control detailsViewerControl = extensionDetailsViewer.getControl();

if (detailsViewerControl != null && !detailsViewerControl.isDisposed())
{
detailsViewerControl.setEnabled(!isReadOnly);
}
}
setListenerEnabled(true);
}


The instance of Tree represents the list of extensions, which is highlighted in the photo below:



And as I mentioned before, removeButton refers to the Delete button which I am trying to fix. At first I believed that perhaps refresh() simply wasn't getting called when switching tabs e.g. clicking on Extensions, then clicking Documentation, then clicking Extensions again.

So I inserted a breakpoint at the beginning of the method and ran a stack trace. Turns out refresh() actually DOES get called whenever the tabs are switched. So why is the button not being re-disabled? This isn't even the real issue, which I'll explain later.

I then realized that even though refresh() was being called after every switch, the method was being exited early. If we look at this code:


if ( prevInput == input)
return;
else
prevInput = input;


Notice here that if "prevInput" is not equal to "input" then the method should return out. I realized that prevInput refers to the previous status of the Extensions frame. It was being compared to the current status of the frame, and because switching tabs wasn't changing the actual contents of the frame, the method was exiting early. To change this, I changed the if statement to look like this:


if ( prevInput != input)
prevInput = input;


When I ran the Eclipse Application, the button behaved properly!



Note that I switched tabs before switching back to the Extensions tab in this picture. When I clicked on an object in the WSDL editor the buttons kept their proper state. It would appear that the problem is solved.

However, to me the real issue is: Why does refresh() need to be called every time to persist the state of the buttons? Why don't the buttons remember their state? I believe that the answer isn't that the buttons are forgetting their state, I believe it's because something else is changing their state. This would suggest that the problem actually doesn't occur in refresh(). The change I made now is a bit of a cheap fix, what I would like to do is eventually find the root of this problem and fix it, which is why I don't plan on submitting a patch yet.

Clearly the developers wanted to make sure that the code executed in refresh() wasn't done every time a tab was switched. Thus, the problem with my solution is that by cutting the old if statement out, I ensure that the code inside refresh() is executed on every tab switch, which is a waste of resources. So for 0.3 my current objective is to find the underlying problem as to why the Delete button's state change isn't being persisted, and that means finding out where else the button is being changed.

1 comment:

pliu said...

Hi, Jesse.

Have you posted a link to this blog posting at Bugzilla? The WTP developers who oversee "your" bug
will be informed immediately.

Peter.