Community comments needed for Common Navigator label providers

Bug 252293 points out an issue that Common Navigator navigatorContent (extensions) (NCEs) are not properly invoked for the purpose of providing a label when the NCE overrides another.

In the course of investigating this problem, I found a few issues in this area where the label provider handling is not really right.  Unfortunately, to make it right, current users of the CNF might somehow be broken.  This could particularly apply to the Java extensions as used the WTP, and to commercial products.

In general, this is really only a problem if you have NCEs that use either triggerPoints or possibleChildren expressions.  If you use enablement expression (which sets both of these), you should not have any of these problems.

Here is the main problem:

You may not get the NCE you are expecting for a label provider.  When an object (in the CommonViewer) is first discovered by an NCE (typically as a result of the triggerPoints/enablement expression), its association with the NCE is cached for performance reasons.  If a label provider subsequently needs to be found for the object, the cache is checked, even though there may be qualifying NCEs that specify possibleChildren (and possibly overrides).  If the object is found in the cache, the cached NCE is used, causing the “correct” NCE(s) to be ignored.  However if the object is not found, the normal search for NCEs will take place.

Bug 255793 proposes an elegant solution to the problem which is to simply always have the label provider NCE be the same as the NCE that provided the content (which is how it actually works much of the time due to the caching, but not always).  However, I don’t see a way it can be implemented without a serious performance problem (if someone has a way, we can consider this, but for now I have closed it).

My proposal: Create a second cache.  The existing cache is used only for triggerPoints/enablement expressions, the second cache is used for possibleChildren expressions.  And also fix the issue in the above bug 252293 that overrides are respected when searching for label providers (and other uses of possibleChildren).  This will make the behavior deterministic and aligned with what is documented (once the doc is fixed, see below).

The downside of this is that it might break existing applications that somehow work with the current behavior.  These is where I would like to see community comment.  Please indicate your pleasure or displeasure in bug 252293. I intend to address this issue in M6, or if the comments are fast and unequivocal, I can address it in M5.

A secondary problem is that the current documentation is bad:

The current documentation for the NCE states that:

The triggerPoints expression describes the elements that will cause this extension to be invoked for either children or for labels. The possibleChildren expression describes the elements that the extension may be able to provide a parent for. Clients should describe all elements that could be set as the selection to ensure that the link with editor support can properly expand to the right node.

… and …

A navigator content extension defines a content provider and label provider that can be used to provide children whenever an element matches the triggerPoints expression and also to provide a parent whenever an element matches the possibleChildren expression.

The above statements are not correct with respect to label providers.  Label providers use the possibleChildren expression, not triggerPoints expression.  The documentation will be corrected to match the current behavior, since changing the behavior to match the documentation could break many things.

Advertisements

About francisu

VP, Architecture at Talend, overall responsible for product architecture and governance. Also maintainer of the Jenkins EC2 plugin.
This entry was posted in Common Navigator. Bookmark the permalink.

2 Responses to Community comments needed for Common Navigator label providers

  1. Francis, would this resolve an annoying issue in Mylyn’s task editor Context page that doesn’t show the proper labels for entries corresponding to Java package names?

  2. Francis Upton says:

    It might. Is there a bug on this somewhere? I don’t recall seeing anything on my list about this. If not, can you or someone file one so we can check on this?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s