Community
Participate
Working Groups
Created attachment 146876 [details] missing labels see the screenshot labels are missing in EAR and EJB project.
Maybe others know if WTP changed in this area ... but, have you tried testing with base Eclipse 3.5? Instead of the near 3.5.1 version? Just in case it is their regression?
I just tested this scenario with eclipse-SDK-3.5.1RC2-win32.zip and eclipse-SDK-3.5.1RC3-win32.zip. (All other files were identical.) Only the setup with the RC3 version of the base showed this problem. I am not sure if this was a change that we were supposed to react to, or if this is a regression from the base. Either way, a change like this between RC2 and RC3 is not desireable. David Williams did some investigation- the most likely candidate for the cause of this is bug 287103
Here is what I discovered investigating the Ear 5 labels - in bug 287103, they altered the way that the label provider is looked up. Instead of going directly to the Ear 5 label provider, it goes through NavigatorContentServiceLabelProvider.getStyledText(), and gets a set of 3 possible label provider extensions- Web 25, Ear 5, and Working Sets. (Note: only Ear 5 has a trigger for ModulesNode and BundledNode... but it wants to check all three now.) It tries Web 25 first. It gets our Web25LabelProvider, which has a org.eclipse.ui.model.WorkbenchLabelProvider to provide labels for Adaptable objects. Since the Web25LabelProvider does not work for either the ModulesNode or BundledNode, it calls WorkbenchLabelProvider.getText(Object element), which returns "" for the text. NavigatorContentServiceLabelProvider.findStyledText(), on line 183, simply checks: if (text != null) And, since "" is not equal to null, the text for many of our items is "". Simply adding a check to see if text.length > 0 to that if statement should remedy our problem.
(In reply to comment #3) > returns "" for the text. > NavigatorContentServiceLabelProvider.findStyledText(), on line 183, simply > checks: > if (text != null) > And, since "" is not equal to null, the text for many of our items is "". > Simply adding a check to see if text.length > 0 to that if statement should > remedy our problem. This is probably the right solution; I will prepare a patch and run our tests.
Kim, is another 3.5.1 rebuild still possible at this point?
The RC4 build isn't due to be released to galielo build until Sept 14 (Mon)
This should be OK for WTP. If possible, please point us an earlier nightly build of the platform that contains the fix, so we can retest before RC4 is declared.
(In reply to comment #7) > This should be OK for WTP. > If possible, please point us an earlier nightly build of the platform that > contains the fix, so we can retest before RC4 is declared. I can put the patch into tonight's HEAD, but I don't think that will do you any good (unless you want to test with HEAD). I will have the patch done sometime today (California time), but I don't think there will be an M build before Monday. Of course you can always just apply the patch and test.
OK, then let's have the patch attached to this bug.
I can't figure out how to make this problem happen. I am using the 3.5.0 eclipse-jee as my target platform and all of the labels are OK, but I can't figure out how to create Jar files under the deployment descriptors. Can you provide exact steps for me to follow to make this happen? The sooner the better as we need lots of approval to get this into a build tonight.
Also, I can't find a regression in the NavigatorContentServiceLabelProvider. When getText() is called on the client label provider, and it returns null, then it moves onto the next. If it returns an empty string, it uses that. By contrast the getStyledText() works differently as there is no way to return a null from getStyledText(). So if it returns an empty string it is bypassed. The comment above in the bug report refers to line 183 in NCSLP which has nothing to do with getting the text (or styled text). I think the change to have getText() skip empty labels is fine if it fixes this problem, but I don't fully understand what change caused the problem in the first place (vs. the 3.4 behaviour).
Ok, I figured it out, creating EJB 3.0 project will show the problem. Trying out the patch now.
Created attachment 146987 [details] Proposed patch This has been tested with the navigator tests, WST and the problem described in this bug report. Will test CDT in a few minutes. DTP should also be tested (we need to ask them to do a good job).
+1 from me for 3.5.1. Not fixing looks really bad, and the fix is low risk. I don't think an empty text for a tree item is ever going to be the right answer (it's really hard to select the item if it doesn't have any text, for example). In the worst case, we will get a label like "Error: no label provider for ..." instead of an empty label, but all examples Francis and I have looked at work well. We will continue testing this, and would appreciate another test pass from the DTP guys. McQ, +1 for 3.5.1 - including a rebuild, hopefully today?
I reviewed the fix with Boris. In the absence of a "real" PMC member and given the urgency of making a fix, I'm adding my "almost PMC" +1. The change is consistent with how we handle the result from IStyledLabelProvider, and I agree an empty string is never a good label in a tree. The regression for WTP is bad enough that a fix at this stage is warranted.
Patch released to R3_5_maintenance. Map file is updated and rebuild has been requested. The build will hopefully happen later today.
I've been following recent work on the Common Navigator and was amazed by the diligence that I saw around unittests and trying extremely hard to get every possible case considered and verified. The fix looks innocent, and most of all I trust Boris and John. +1 from a "real" PMC member, just for the records :)
(In reply to comment #17) > I've been following recent work on the Common Navigator and was amazed by the > diligence that I saw around unittests and trying extremely hard to get every > possible case considered and verified. Thanks Martin, we still need a test case for this one too, but we will have it soon. It makes a big difference to hear that the work is appreciated.
Dimitar, David, Carl - thanks for finding this issue just in time, and for the detailed analysis. Francis - thanks for the extremely short turnaround time. Not sure if everyone on this bug knows it, but Francis is maintaining the Common Navigator on his own time, without funding from those who benefit from his work, in true open source spirit. Happy birthday, Francis!
Brian, can you and your folks test tonights M build (that has this fix) against DTP? I don't anticipate problems, but we should have our bases covered.
Created attachment 147036 [details] Patch as applied to HEAD This patch is a little more comprehensive than the one applied to 3.5.1 in that both instances where getText() is called and returns "" is handled correctly (by skipping the label provider). This patch also adds test cases that show the problem and have some additional tests to make sure returning null is also correctly handled.
Released to HEAD (3.6M2)
Verified in M20090911-1628
Verified that Franck's new tests (bug 287261) work with this change.
I verified the fix submitted in the R3_5_maintenance branch. Francis, thank you for the efforts and quick resolution! I wish you good luck and prosperity and I hope we haven't ruined your birthday party!
Thanks for fixing this Francis! Happy belated Birthday!
Hi Team, This changed has caused a significant regression in our GMF based editor. We have GMF elements that are not yet named in the project explorer. The elements have a blank name and icon. Elements are created and subsequently the user can set a name. In some cases however, these elements are never given a name and "" always shows in the project explorer. in the newer target, we now check for a zero length string and returns null: String text= labelProvider.getText(anElement); if (text != null && text.length() > 0) { return new StyledString(text); } } return null; The getStyledText(...) method gets the null value and returns the error message that we see CommonNavigatorMessages.NavigatorContentServiceLabelProvider_Error_no_label_provider_for_0_
Anthony, please open a new bug report. This problem is considered fixed in 3.5.1 and if it caused another issue then we should track it in a separate bug. Thanks.
(In reply to comment #28) > Anthony, please open a new bug report. This problem is considered fixed in > 3.5.1 and if it caused another issue then we should track it in a separate bug. > Thanks. OK, Bug 296253 Submitted Just to be clear, the problem is not fixed, having no label but an icon in project explorer is a valid use case.
>OK, Bug 296253 Submitted Thanks, Anthony. >Just to be clear, the problem is not fixed, having no label but an icon in >project explorer is a valid use case. I'm fully with you. I tried to explain that but it was not listened to my concerns (see bug 262883) and then I finally gave up.
Changing back to verified, as this fix was verified as working as intended. (yes, it broke something else, but that's the subject of bug 296253)
