Community
Participate
Working Groups
HEAD after fix for bug 366063 does not support workspace/project-wide nullness defaults. We need to investigate a configurable option to warn in the following cases: 1) For named packages, if the package-info.java does not contain the @NonNullByDefault annotation, or if it contains no package-info.java, warn at exactly one class inside the package 2) For default packages, warn at each class inside the package which does not contain the @NonNullByDefault annotation. The warning message can be "A default null annotation has not been specified for the package {0}" and "A default null annotation has not been specified for the type {0}"
This fix should replace JavaCore#COMPILER_NONNULL_IS_DEFAULT by an option that states an intention, but doesn't change behavior, e.g. COMPILER_PB_MISSING_NONNULL_BY_DEFAULT_ANNOTATION.
And the option should not be boolean but with values error/warning/ignore.
Created attachment 211418 [details] proposed fix This patch warns in the following cases: (1) Package-info.java exists and does not have any default nullness annotation: warn at ONLY package-info.java (2) Package-info.java does not exist inside a named package: warn at the package declaration site in each type inside the package For 1 and 2, warning is "A default nullness annotation has not been specified for the package {0}" (3) Default package: Warn (blaming the type) at each type that does not contain a default nullness annotation. Warning is "A default nullness annotation has not been specified for the type{0}" For (2), Srikanth suggested to warn at only 1 type instead of all types. However, asssigning a special bit to the package binding to signal that it has been warned against does not quite work for the incremental builder because the binding is re-created for each type and the warning re-appears. The only solution here may be to store the bindings of already warned packages in lookup environment. However, in the current scenario where you see warnings on all types, the only fix possible is to put the annotation by creating a package-info.java which will wipe out all warnings in that package. So, I'm not too motivated to go beyond what's in the current patch. Srikanth/Stephan, any issues with the above conclusion? (Patch still needs regression tests)
(In reply to comment #3) > binding is re-created for each type and the warning re-appears. The only > solution here may be to store the bindings of already warned packages in lookup > environment. Why won't we do that ? Producing identical warnings for every type in the package is not very useful ? On large projects this could produce thousands of warnings where we could do away with just a few dozens of them ? > (3) Default package: Warn (blaming the type) at each type that does not contain > a default nullness annotation. Warning is > "A default nullness annotation has not been specified for the type{0}" Does it make sense to restrict this warning only for top level types ?
(In reply to comment #4) > Why won't we do that ? Producing identical warnings for every type in the > package is not very useful ? On large projects this could produce thousands > of warnings where we could do away with just a few dozens of them ? The point here is for the user to actually notice that warning. Having it on one type in a package with 100 types may mean that it might skip the field of view of the user. Having the warning on all types is more consistent. In that sense, by doing the extra work to restrict the warning on one type does not offer a great advantage. Anyway, when there's a package-info, the warning comes only on the package-info. Markus, what's your opinion on this? > Does it make sense to restrict this warning only for top level types ? Yes, and thats what the patch already does.
Created attachment 211472 [details] proposed fix v1.0 + regression tests Complete patch. Batch COmpiler option -nonNullByDefault has been replaced by "missingDefaultAnnot"
In bug 366063 I argued that reporting a warning against a package would require a few infrastructural changes. I'm not sure if/how this influenced the current design to report problems against types in bogus packages. However, if no agreement can be found, how to exactly report using only types as the point of reference, shouldn't we indeed have the discussion how reporting against a package could be implemented? Just want to be sure that my words of warning don't block a better solution. Sorry for being late with these remarks.
I like the error on the package-info.java if the file already exists. Otherwise, I'd prefer an error on the package. If that's too hard, the second-best would be an error on any single type of the package (preferably the lexically first one). As last resort, an error in every CU would also be acceptable if the other solutions turn out to be too expensive.
(In reply to comment #5) > The point here is for the user to actually notice that warning. Having it on > one type in a package with 100 types may mean that it might skip the field of > view of the user. I don't buy that argument. A user who cares about this will set the problem to error. In that case, a flood of errors on all CUs is counterproductive.
Created attachment 211800 [details] patch to warn on package only Here's an experiment for warning on package and not its CU's, in case there's no package-info.java. There are a couple of problems with this though: 1) Somehow I see 2 warnings for each problem on the package resource, not sure where the duplicate warning comes from. 2) When the user corrects the issue by creating a package-info.java and putting the default annotation there, the incremental builder doesn't rebuild the CU's against which the problem was originally recorded. Hence, the marker does not get deleted until a full build occurs. Not sure how to solve these problems. Stephan, do you have any ideas? I'm guessing the first problem can be solved, but second one is going to be a dead-end.
(In reply to comment #10) > Created attachment 211800 [details] > patch to warn on package only Relevant changes are in org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.storeTasksFor(SourceFile, CategorizedProblem[]) and org.eclipse.jdt.internal.compiler.problem.ProblemHandler.handle(int, String[], int, String[], int, int, int, ReferenceContext, CompilationResult)
(In reply to comment #10) > Created attachment 211800 [details] > patch to warn on package only > > Here's an experiment for warning on package and not its CU's, in case there's > no package-info.java. > There are a couple of problems with this though: > 1) Somehow I see 2 warnings for each problem on the package resource, not sure > where the duplicate warning comes from. > 2) When the user corrects the issue by creating a package-info.java and putting > the default annotation there, the incremental builder doesn't rebuild the CU's > against which the problem was originally recorded. This also means this incremental build is inconsistent, right? Sounds like we should feed more cus into the next build iteration? This change would be necessary independtly of how we report errors, right? > Hence, the marker does not get deleted until a full build occurs. > > Not sure how to solve these problems. Stephan, do you have any ideas? I'm > guessing the first problem can be solved, but second one is going to be a > dead-end. I can take a look later today or tomorrow.
(In reply to comment #10) > 2) When the user corrects the issue by creating a package-info.java and putting > the default annotation there, the incremental builder doesn't rebuild the CU's > against which the problem was originally recorded. Hence, the marker does not > get deleted until a full build occurs. If package-info.java exists already and an annotation is added now, does incremental rebuild files properly ? (It should as this is something that got fixed during bug 214948) Is it only when package-info is created freshly that you are observing an anamalous behavior ?
(In reply to comment #13) > If package-info.java exists already and an annotation is added now, does > incremental rebuild files properly ? (It should as this is something > that got fixed during bug 214948) Is it only when package-info is created > freshly that you are observing an anamalous behavior ? Exactly. Thats because the error was reported on the contained CU's, and adding the package-info.java doesn't trigger their build.
(In reply to comment #14) > (In reply to comment #13) > > If package-info.java exists already and an annotation is added now, does > > incremental rebuild files properly ? (It should as this is something > > that got fixed during bug 214948) Is it only when package-info is created > > freshly that you are observing an anamalous behavior ? > Exactly. Thats because the error was reported on the contained CU's, and adding > the package-info.java doesn't trigger their build. OK, then this is something orthogonal to the current issue. We need to make sure that if observe a delta that indicates that a package-info file got created (and if there are package level annotations), then the package should be rebuilt.
(In reply to comment #15) > OK, then this is something orthogonal to the current issue. > We need to make sure that if observe a delta that indicates > that a package-info file got created (and if there are package > level annotations), then the package should be rebuilt. I was wrong. We do build all the types in the package correctly. The problem is that the markers which were created for the package resource do not get deleted on an incremental build, and so the problems view still shows those errors that are now actually fixed. We need to find a way to delete markers on package resource as soon as package-info.java is created.
I'd say we need to remove all package-problems when package-info.java is compiled, right? I tried this snippet: if (CharOperation.equals(sourceFile.getMainTypeName(), TypeConstants.PACKAGE_INFO_NAME)) { IResource pkgResource = sourceFile.resource.getParent(); JavaBuilder.removeProblemsFor(pkgResource); } assuming we have this problem only for incremental builds the correct location would be as the first statement in IncrementalImageBuilder.updateProblemsFor() From quick experiments in a runtime workbench this seems to do the trick.
(In reply to comment #17) > I'd say we need to remove all package-problems when package-info.java is > compiled, right? > > I tried this snippet: > > if (CharOperation.equals(sourceFile.getMainTypeName(), > TypeConstants.PACKAGE_INFO_NAME)) { > IResource pkgResource = sourceFile.resource.getParent(); > JavaBuilder.removeProblemsFor(pkgResource); > } Yes, I was playing around with resource.deleteMarkers instead of JavaBuilder.removeProblemsFor. However, I don't know why, when I press space in a file with no package-info and save, a new "missing default annotation" problem appears, and the list keeps on increasing with every subsequent save. Couldn't yet find out the cause for this.
(In reply to comment #18) > (In reply to comment #17) > > I'd say we need to remove all package-problems when package-info.java is > > compiled, right? > > > > I tried this snippet: > > > > if (CharOperation.equals(sourceFile.getMainTypeName(), > > TypeConstants.PACKAGE_INFO_NAME)) { > > IResource pkgResource = sourceFile.resource.getParent(); > > JavaBuilder.removeProblemsFor(pkgResource); > > } > Yes, I was playing around with resource.deleteMarkers instead of > JavaBuilder.removeProblemsFor. OK, for a marker on a package, which actually corresponds to a resource, going directly to resource.deleteMarkers might be enough. > However, I don't know why, when I press space in > a file with no package-info and save, a new "missing default annotation" > problem appears, and the list keeps on increasing with every subsequent save. > Couldn't yet find out the cause for this. If I understand correctly, so far you ensure uniqueness of the marker only within the context of one build, right? So each build creates a new marker, as it doesn't see that one already exists, right? So in AbstractImageBuilder.storeProblemsFor(), right when you detect IProblem.MissingNonNullByDefaultAnnotationOnPackage and lookup the package resource, why not first look if a corresponding marker already exists (and skip this problem if so)? S.t. like: IMarker[] existingMarker = resource.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_ZERO); int len = existingMarker.length; for (int j=0; j<len; j++) { if (((Integer)existingMarker[j].getAttribute(IJavaModelMarker.ID)).intValue() == IProblem.MissingNonNullByDefaultAnnotationOnPackage) { continue problems; // marker already present } } (I added "problems" as a label for the outer loop).
(In reply to comment #19) > So in AbstractImageBuilder.storeProblemsFor(), right when you detect > IProblem.MissingNonNullByDefaultAnnotationOnPackage and lookup the package > resource, why not first look if a corresponding marker already exists (and skip > this problem if so)? S.t. like: Oh, so for CU's those markers don't get recreated because of the removeProblemsFor(..) call in org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder.updateProblemsFor(SourceFile, CompilationResult). Hmm, makes sense.
Created attachment 211945 [details] proposed fix v2.0 + regression tests This fix includes fix for warning on package instead of its CU's as well as fixes for the incremental build. The problem marker is only created once for each package (ProblemHandler) , and deleted as soon as a package-info.java is added(IncrementalImageBuilder). Running all Junits.
Created attachment 211956 [details] proposed fix v2.1 + regression tests This patch - fixes a small issue with file separator for calculating package fragment in ProblemHandler - for batch compiler, it makes the warning come back on the CU's of a package to warn for missing default annotation. - updates a couple of tests (BatchCOmpilerTest and CompilerInvocationTests)
Created attachment 211957 [details] doc patch
(In reply to comment #22) > - for batch compiler, it makes the warning come back on the CU's of a package > to warn for missing default annotation. Does anyone have any issue with this? On the command line, it makes more sense to warn against a tangible line in the source. Also, I did not think it was worth the effort to make the warning appear on only 1 CU here, since this is a special case for the batch compiler.
Stephan, please give this patch a final once-over, thanks.
I still see a few issues: (1) (In reply to comment #21) > [...] The problem marker is only created once for > each package (ProblemHandler) Blocker: Sorry, but I think these classes are off limits for the compiler which cannot depend on the model. Seems like these issues must be handled in the builder, or would that cause trouble for the batch compiler? (2) Discussion: From reading SourceTypeBinding.evaluateNullAnnotations() it seems that you'll report a missing default even if all types in a package do have a default annotation. Is that intended? Won't users expect that annotating all types should be sufficient? Simply changing the order of checks inside said method could achieve this change in semantics. (3) Javadoc of COMPILER_PB_MISSING_NONNULL_BY_DEFAULT_ANNOTATION contains a little stutter. May need to align doc with result of discussion (2). (4) Nitpick: Same method evaluateNullAnnotations(): name of local variable "isDefault" is ambiguous in this context (5) Tests: I couldn't find any tests checking specifically for the marker on a package. Maybe some should be added to NullAnnotationModelTests? (Or just add an additional check to existing tests).
(In reply to comment #26) > Blocker: Sorry, but I think these classes are off limits for the compiler which > cannot depend on the model. Seems like these issues must be handled in the > builder, or would that cause trouble for the batch compiler? Hmm, you're right. But then I'd have to create a CategorizedProblem for each type in the package, and they cannot be of IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER category because then they will appear on the editor in the CU's. Maybe creating a new category along with some UI work will help? I'm not sure how that works, can try. Will have to think of a way to create only 1 categorized problem for each package. Sigh. > (2) > Discussion: From reading SourceTypeBinding.evaluateNullAnnotations() it seems > that you'll report a missing default even if all types in a package do have a > default annotation. Is that intended? Won't users expect that annotating all > types should be sufficient? Simply changing the order of checks inside said > method could achieve this change in semantics. Yes, I meant to do that when the other things were working but slipped out of my mind. > (5) > Tests: I couldn't find any tests checking specifically for the marker on a > package. Maybe some should be added to NullAnnotationModelTests? (Or just add > an additional check to existing tests). AnnotationDependencyTests has a test for that case
(In reply to comment #27) > (In reply to comment #26) > > Blocker: Sorry, but I think these classes are off limits for the compiler which > > cannot depend on the model. Seems like these issues must be handled in the > > builder, or would that cause trouble for the batch compiler? > Hmm, you're right. But then I'd have to create a CategorizedProblem for each > type in the package, and they cannot be of > IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER category because then they will > appear on the editor in the CU's. Maybe creating a new category along with some > UI work will help? I'm not sure how that works, can try. Will have to think of > a way to create only 1 categorized problem for each package. Sigh. Do these problems still exist if we opt for a simple `report against the first CU in the package' approach ? Such a solution would surely eliminate the need for any special treatment for the batch compiler, may also eliminate the other complications we are discussing ? Not sure if we are over-engineering here.
(In reply to comment #28) > (In reply to comment #27) > > (In reply to comment #26) > > > Blocker: Sorry, but I think these classes are off limits for the compiler which > > > cannot depend on the model. Seems like these issues must be handled in the > > > builder, or would that cause trouble for the batch compiler? > > Hmm, you're right. But then I'd have to create a CategorizedProblem for each > > type in the package, and they cannot be of > > IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER category because then they will > > appear on the editor in the CU's. Maybe creating a new category along with some > > UI work will help? I'm not sure how that works, can try. Will have to think of > > a way to create only 1 categorized problem for each package. Sigh. > > Do these problems still exist if we opt for a simple `report against the first > CU in the package' approach ? Such a solution would surely eliminate the need > for > any special treatment for the batch compiler, may also eliminate the other > complications we are discussing ? Not sure if we are over-engineering here. I would think of a four-step process: - compiler remembers in PackageBinding.tagBits when this problem has been created -> report at most once per build - problem is attached to the current CU - builder detects these problems (either by problemId or by category) and when creating the marker redirects that marker to the package - builder only adds a new marker if none of the same kind is already found (see comment 19) -> avoid duplicates from subsequent builds I think most of this exists or existed in the patch plus previous comments.
(In reply to comment #29) > - builder detects these problems (either by problemId or by category) and when > creating the marker redirects that marker to the package > - builder only adds a new marker if none of the same kind is already found (see > comment 19) -> avoid duplicates from subsequent builds Are these last two steps absolutely necessary ? Sorry I have missed out some discussion/context here.
(In reply to comment #30) > (In reply to comment #29) > > > - builder detects these problems (either by problemId or by category) and when > > creating the marker redirects that marker to the package > > - builder only adds a new marker if none of the same kind is already found (see > > comment 19) -> avoid duplicates from subsequent builds > > Are these last two steps absolutely necessary ? Sorry I have missed out > some discussion/context here. They'd implement the change from marking a random CU towards marking the package. This needs to happen outside the compiler because the compiler can't see IPackageFragment. The last item fixes the problem observed in comment 18. Part of this existed in the patch in comment 10, see the change in AbstractImageBuilder.
(In reply to comment #31) > They'd implement the change from marking a random CU towards marking the > package. This needs to happen outside the compiler because the compiler can't > see IPackageFragment. I see. I was questioning the need to do this. Could we be lazy and report it against some unique CU per package per build loop and get away with it ?
(In reply to comment #32) > (In reply to comment #31) > > > They'd implement the change from marking a random CU towards marking the > > package. This needs to happen outside the compiler because the compiler can't > > see IPackageFragment. > > I see. I was questioning the need to do this. Could we be lazy and report > it against some unique CU per package per build loop and get away with it ? Ayush and me where trying to find a solution to comply with Markus' request: (In reply to comment #8) > I like the error on the package-info.java if the file already exists. > > Otherwise, I'd prefer an error on the package. > If that's too hard, the second-best would be an error on any single type of the > package (preferably the lexically first one). > As last resort, an error in every CU would also be acceptable if the other > solutions turn out to be too expensive. I think we're close to the preferred solution. Still, if "close" isn't close enough considering the time line, we may have to revert to the "second-best" solution, which may not be easier, which would then take us to the "last resort". Necessity is relative :)
(In reply to comment #32) > I see. I was questioning the need to do this. Could we be lazy and report > it against some unique CU per package per build loop and get away with it ? A problem with this is also that everytime we compile only a single CU, we don't know whether an earlier CU has already triggered this warning. Saving this info in the package binding also won't help since the binding is not persisted across two separate builds (eg. p.A already has warned, now I create p.B - the warning comes on both A and B) (In reply to comment #29) > - builder detects these problems (either by problemId or by category) and when > creating the marker redirects that marker to the package The problem here is that even when the builder redirects creation of the marker to the package, the CU open in the editor still shows this warning, even though there are no markers on the CU's in the package explorer. (Maybe the editor picks it directly from the unit.) In fact, I like this - the warning marker remains only on the package, but the open editor still shows it on the package declaration, enabling the user to employ the quick fix right there. Stephan, whats your take on this?
(In reply to comment #34) > (In reply to comment #32) > > I see. I was questioning the need to do this. Could we be lazy and report > > it against some unique CU per package per build loop and get away with it ? > A problem with this is also that everytime we compile only a single CU, we > don't know whether an earlier CU has already triggered this warning. Saving > this info in the package binding also won't help since the binding is not > persisted across two separate builds (eg. p.A already has warned, now I create > p.B - the warning comes on both A and B) That's why I mentioned item 4: "- builder only adds a new marker if none of the same kind is already found (see comment 19) -> avoid duplicates from subsequent builds" Placing the snippet from comment 19 into the builder worked OK in my workspace. The trick with PackageBinding.tagBits only helps for: - batch compilation - minimizing the creation of unnecessary CategorizedProblems > (In reply to comment #29) > > > - builder detects these problems (either by problemId or by category) and when > > creating the marker redirects that marker to the package > The problem here is that even when the builder redirects creation of the marker > to the package, the CU open in the editor still shows this warning, even though > there are no markers on the CU's in the package explorer. (Maybe the editor > picks it directly from the unit.) > In fact, I like this - the warning marker remains only on the package, but the > open editor still shows it on the package declaration, enabling the user to > employ the quick fix right there. Stephan, whats your take on this? Wait, that's the reconciler, right? So, it won't spam the ProblemsView? In that case we might indeed just leave that part as you say, I like that, despite the little inconsistency. If we change our mind on this, a simple change in CompilationUnitProblemFinder.process() should buy us any desired behavior.
Created attachment 212134 [details] proposed fix v3.0 + regression tests Let me attach the patch to better answer your question. The Problems View is not polluted, and even the package explorer isn't. Only the open editor shows the warning. You can test the patch to see the new behaviour (which is same as previous patch's behaviour, except that now there's a warning ONLY in the open editor.) (Tests may still need an update)
(In reply to comment #35) > The trick with PackageBinding.tagBits only helps for: > - batch compilation > - minimizing the creation of unnecessary CategorizedProblems This is handled in the patch by marking the default nullness of the package as NULL_UNSPECIFIED_DEFAULT as soon as warning is reported.
(In reply to comment #36) > Created attachment 212134 [details] > proposed fix v3.0 + regression tests In IncrementalImageBuilder where we delete markers on package, the depth should be IResource.DEPTH_ZERO and not DEPTH_INFINITE. I will also take care of points 3 and 4 of comment 26, and then the patch should be good to go
(In reply to comment #33) > I think we're close to the preferred solution. Still, if "close" isn't close > enough considering the time line, Great, I'll let you two decide. I thought we were not converging, but that seems to be incorrect, so we are good.
Using fix v3.0 I only see one issue remaining: When adding a default annotation to a type where the package lacks such default, during incremental compilation the problem is wrongly removed from the package. However, this can easily be fixed just by removing the else block in IncrementalImageBuilder.updateProblemsFor(): for resolving this problem users have only two options: add/edit package-info.java or change the preference. The latter would require a full build to take effect, so only actions involving package-info.java need to be handled during incremental compilation. Judging from this status plus the planned changes mentioned in comment 38 this patch is essentially ready to go. Since I will be offline for a few days I'm giving my +1 trusting Ayush to walk the last yards to finish. Here are a few final, but not vital remarks: (a) Some reminders regarding stale changes that are no longer needed: - a block in ProblemReporter - all changes in PackageFragment and Util (b) BatchCompilerTest contains bogus references to bug 325342. Also the combination with a syntax error looks unmotivated to me.
(In reply to comment #40) > Using fix v3.0 I only see one issue remaining: When adding a default annotation > to a type where the package lacks such default, during incremental compilation > the problem is wrongly removed from the package. However, this can easily be > fixed just by removing the else block in > IncrementalImageBuilder.updateProblemsFor(): for resolving this problem users > have only two options: add/edit package-info.java or change the preference. The > latter would require a full build to take effect, so only actions involving > package-info.java need to be handled during incremental compilation. Actually the else block there was put intentionally because there's one case which would break otherwise: 1) Types A and B exist in p. B has default annotation but A doesn't. 2) Now I add default annotation to A -> incremental build happens -> But marker is not removed and we still get the missing default annotation warning on package even though all types in it have a default annotation. We want this case to be silent right? Thats why its necessary. Let me know if i'm missing something. [..] > (a) Some reminders regarding stale changes that are no longer needed: > - a block in ProblemReporter > - all changes in PackageFragment and Util > (b) BatchCompilerTest contains bogus references to bug 325342. Also the > combination with a syntax error looks unmotivated to me. Yeah, will clean this up. Thanks a lot once again for your patient reviews and ideas!
(In reply to comment #41) > (In reply to comment #40) > Actually the else block there was put intentionally because there's one case > which would break otherwise: > 1) Types A and B exist in p. B has default annotation but A doesn't. > 2) Now I add default annotation to A -> incremental build happens -> But marker > is not removed and we still get the missing default annotation warning on > package even though all types in it have a default annotation. We want this > case to be silent right? > Thats why its necessary. Let me know if i'm missing something. Outch, you're right. I remember. Now that's quite a riddle :) So we want the last one leaving to turn off the light: the last CU lacking a default, when the default is added remove the warning. How can incremental compilation know this? A custom marker attribute comes to mind that counts the number of CUs lacking a default, with proper incr/decr operations during incremental build. But that sounds like an expensive solution. So, if you don't want to go that road, I would be fine with going back one little step and go on issuing the warning, even if all types in the package have a type level default. The semantics would be: warn if any *package* lacks a default - so that would be clean, we'll just have to cope with those users that will be surprised that type level defaults don't suffice. Mh, still better than the randomness during incremental compilation, I guess.
(In reply to comment #42) > So, if you don't want to go that road, I would be fine with going back one > little step and go on issuing the warning, even if all types in the package > have a type level default. The semantics would be: warn if any *package* lacks > a default - so that would be clean, we'll just have to cope with those users > that will be surprised that type level defaults don't suffice. Mh, still better > than the randomness during incremental compilation, I guess. Yes this matches what Srikanth and I discussed in the beginning and also fixes bug 373405. ;)
Released jdt.core part via commit a234173ab4d645c8990c97cafa00ec82ce0a909f
Released doc patch via http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/?id=9f135bcee4d4a35fc4076870014073f697e544c3
I have raised three FUPs: bug 374063 (Core), bug 374122 (Doc) and bug 374111 (UI); other than that the fix works as desired. Verified for 3.8 M6 using build I20120312-1800.
