Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 318224

Summary: [Decorators] Decoration context should specify decoration area in image (was: Model-based synchronize view shows decorations at unexpected location)
Product: [Eclipse Project] Platform Reporter: Markus Keller <markus.kell.r>
Component: UIAssignee: Platform UI Triaged <platform-ui-triaged>
Status: CLOSED WONTFIX QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, prakash
Version: 3.7Keywords: api
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard: stalebug
Bug Depends on: 314283    
Bug Blocks:    
Attachments:
Description Flags
Fix idea
none
Screenshot
none
Screenshot 2 none

Description Markus Keller CLA 2010-06-28 14:33:57 EDT
N20100627-2000

Model-based synchronize view shows decorations at an unexpected location.

To reproduce, e.g. try to commit this Java type to CVS while 
"Preferences > Label Decorations > Java Type Indicator" is enabled:

package xy;
/**
 * @deprecated bad stuff
 */
/*package*/ interface E {
    public void foo();
}


In the Changes tree, the icon for the file gets a big dark "=>" arrow indicating an outgoing change. On top of that, the Java Type Indicator overlays are rendered (the "I" in a purple circle, the / for deprecated and the blue triangle for a package-visible type). I would expect these overlays to be rendered on top of the Java file icon on the left, and not right-aligned.

This has already been a problem in 3.6 for the "interface" adornment, but it became even more visible in 3.7 with the fix for bug 314283.

Below is a stack trace that shows where the 16x16 image from the JDT UI label provider is converted into a 16x22 image. Further down in the stack, the DecoratingStyledCellLabelProvider.update(..) decorates the 16x22 image with the decorators registered in the platform's DecoratorManager.

Debugging hint: To see where the label provider gets initialized, set a breakpoint in CommonViewer#init(). Later the label provider is being enhanced in the constructor of CommonViewerAdvisor.

I guess the CommonViewerAdvisor is the easiest point where this bug could be attacked. The DecoratingStyledCellLabelProvider could take a context property that determines the bounds to which the decorations should be applied. Those bounds will be passed town into the DecoratorManager, which passes them to DecorationScheduler#decorateWithOverlays(..), which passes it to DecorationResult#decorateWithOverlays(..), which can finally call a new constructor of DecorationOverlayIcon that can take the requested bounds into account.


Stack:
[..] at org.eclipse.compare.CompareConfiguration.getImage(CompareConfiguration.java:334)
	at org.eclipse.team.internal.ui.synchronize.ImageManager.getImage(ImageManager.java:94)
	at org.eclipse.team.ui.synchronize.AbstractSynchronizeLabelProvider.getCompareImage(AbstractSynchronizeLabelProvider.java:129)
	at org.eclipse.team.ui.synchronize.AbstractSynchronizeLabelProvider.getCompareImage(AbstractSynchronizeLabelProvider.java:119)
	at org.eclipse.team.ui.synchronize.AbstractSynchronizeLabelProvider.decorateImage(AbstractSynchronizeLabelProvider.java:80)
	at org.eclipse.team.ui.synchronize.AbstractSynchronizeLabelProvider.getImage(AbstractSynchronizeLabelProvider.java:51)
	at org.eclipse.team.ui.mapping.SynchronizationLabelProvider.getImage(SynchronizationLabelProvider.java:115)
	at org.eclipse.ui.internal.navigator.NavigatorContentServiceLabelProvider.findImage(NavigatorContentServiceLabelProvider.java:197)
	at org.eclipse.ui.internal.navigator.NavigatorContentServiceLabelProvider.getColumnImage(NavigatorContentServiceLabelProvider.java:105)
	at org.eclipse.ui.internal.navigator.NavigatorContentServiceLabelProvider.getImage(NavigatorContentServiceLabelProvider.java:98)
	at org.eclipse.ui.internal.navigator.NavigatorDecoratingLabelProvider$StyledLabelProviderAdapter.getImage(NavigatorDecoratingLabelProvider.java:60)
	at org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider.getImage(DelegatingStyledCellLabelProvider.java:184)
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.getImage(DecoratingStyledCellLabelProvider.java:167)
	at org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider.getImage(DelegatingStyledCellLabelProvider.java:184)
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.getImage(DecoratingStyledCellLabelProvider.java:167)
	at org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider.update(DelegatingStyledCellLabelProvider.java:118)
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.update(DecoratingStyledCellLabelProvider.java:134)
	at org.eclipse.jface.viewers.ViewerColumn.refresh(ViewerColumn.java:152)
	at org.eclipse.jface.viewers.AbstractTreeViewer.doUpdateItem(AbstractTreeViewer.java:934)
	at org.eclipse.jface.viewers.AbstractTreeViewer$UpdateItemSafeRunnable.run(AbstractTreeViewer.java:102)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:49)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:175)
	at org.eclipse.jface.viewers.AbstractTreeViewer.doUpdateItem(AbstractTreeViewer.java:1014)
	at org.eclipse.jface.viewers.StructuredViewer$UpdateItemSafeRunnable.run(StructuredViewer.java:481)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:49)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:175)
	at org.eclipse.jface.viewers.StructuredViewer.updateItem(StructuredViewer.java:2141)
	at org.eclipse.jface.viewers.AbstractTreeViewer.createTreeItem(AbstractTreeViewer.java:829)
[..]
	at org.eclipse.team.internal.ui.mapping.CommonViewerAdvisor$NavigableCommonViewer.inputChanged(CommonViewerAdvisor.java:84)
	at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:274)
	at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1664)
	at org.eclipse.team.internal.ui.mapping.CommonViewerAdvisor.setInitialInput(CommonViewerAdvisor.java:359)
	at org.eclipse.team.internal.ui.synchronize.AbstractSynchronizePage.createChangesViewer(AbstractSynchronizePage.java:132)
	at org.eclipse.team.internal.ui.synchronize.AbstractSynchronizePage.createControl(AbstractSynchronizePage.java:113)
	at org.eclipse.team.ui.synchronize.ParticipantPageCompareEditorInput.createPage(ParticipantPageCompareEditorInput.java:135)
	at org.eclipse.team.ui.PageCompareEditorInput.createStructureInputPane(PageCompareEditorInput.java:100)
	at org.eclipse.compare.CompareEditorInput.createOutlineContents(CompareEditorInput.java:643)
	at org.eclipse.compare.CompareEditorInput.createContents(CompareEditorInput.java:539)
	at org.eclipse.team.ui.synchronize.ParticipantPageCompareEditorInput.createContents(ParticipantPageCompareEditorInput.java:316)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizardCommitPage.createChangesPage(CommitWizardCommitPage.java:223)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizardCommitPage.createChangesArea(CommitWizardCommitPage.java:204)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizardCommitPage.createControl(CommitWizardCommitPage.java:126)
	at org.eclipse.jface.wizard.Wizard.createPageControls(Wizard.java:170)
	at org.eclipse.jface.wizard.WizardDialog.createPageControls(WizardDialog.java:734)
	at org.eclipse.jface.wizard.WizardDialog.createContents(WizardDialog.java:606)
	at org.eclipse.jface.window.Window.create(Window.java:431)
	at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1089)
	at org.eclipse.jface.window.Window.open(Window.java:790)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizard.open(CommitWizard.java:432)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizard.run(CommitWizard.java:425)
	at org.eclipse.team.internal.ccvs.ui.wizards.CommitWizard.run(CommitWizard.java:353)
	at org.eclipse.team.internal.ccvs.ui.mappings.AbstractCommitAction.execute(AbstractCommitAction.java:68)
[..]
Comment 1 Markus Keller CLA 2010-06-28 14:43:36 EDT
Created attachment 172937 [details]
Fix idea

Here's a fix idea. It works, but needs some cleanup and new APIs from Platform UI.
Comment 2 Markus Keller CLA 2010-06-28 14:47:40 EDT
Created attachment 172938 [details]
Screenshot

Screenshot with HEAD (bad) and with patch (better).
Comment 3 Dani Megert CLA 2010-07-01 08:36:10 EDT
The 'better' case also looks unreadable to me (like screen cheese). The result must be way better to justify API additions. Besides that I see two other solutions:
- revert the changes made for the Java type indicator
- change Team label provider to not touch the base image but add the decoration
  via real decorator
Comment 4 Markus Keller CLA 2010-07-01 08:41:20 EDT
Created attachment 173210 [details]
Screenshot 2

In HEAD of jdt.ui, I've removed some white pixels around the package-visible overlay, so that the Team overlay is even better visible in the corner case of
default-visible non-class types.
Comment 5 Dani Megert CLA 2010-07-01 08:44:50 EDT
It also makes the error/warning less readable since they get partially obscured by the deprecation overlay. At this point I'd rather revert the Java type indicator additions.
Comment 6 Markus Keller CLA 2010-07-01 09:30:57 EDT
> - revert the changes made for the Java type indicator

That would still leave wrongly rendered decorations for non-class types.

> - change Team label provider to not touch the base image but add the
> decoration via real decorator

Team would have to do both, since the sync direction overlay should already be shown initially. And the contributed platform decorators have the problem that they always show up on the Preference page and can be turned off.


> At this point I'd rather revert the Java type indicator additions.

I disagree, but that should be discussed in bug 314283. Also note that this decorator is off by default.
Comment 7 Markus Keller CLA 2011-03-02 06:13:28 EST
Sorry, I slipped this, and now it's too late for 3.7.

Moving to Platform UI for consideration in 3.8, since the interesting change is really new API in the decorations area.
Comment 8 Eclipse Genie CLA 2019-10-20 04:12:11 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.