292330 – [framework] HighlighterImageDescriptor should not hold on to image

Bug 292330 - [framework] HighlighterImageDescriptor should not hold on to image

Summary: [framework] HighlighterImageDescriptor should not hold on to image

Status:	RESOLVED FIXED

Alias:	None

Product:	z_Archived
Classification:	Eclipse Foundation
Component:	Mylyn (show other bugs)
Version:	unspecified
Hardware:	PC Linux

Importance:	P2 critical (vote)
Target Milestone:	3.4
Assignee:	Steffen Pingel
QA Contact:

URL:
Whiteboard:
Keywords:	noteworthy

Depends on:
Blocks:

Reported:	2009-10-14 19:36 EDT by Steffen Pingel
Modified:	2010-02-03 15:50 EST (History)
CC List:	4 users (show)

See Also:

Attachments
committed patch (5.88 KB, patch) 2009-12-09 23:27 EST, Steffen Pingel	no flags	Details \| Diff
mylyn/context/zip (6.71 KB, application/octet-stream) 2009-12-09 23:27 EST, Steffen Pingel	no flags	Details
proposed patch (55.08 KB, patch) 2009-12-09 23:32 EST, Steffen Pingel	no flags	Details \| Diff
updated patch (56.44 KB, patch) 2010-02-03 15:49 EST, Steffen Pingel	no flags	Details \| Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Steffen Pingel

2009-10-14 19:36:37 EDT

HighlighterImageDescriptor caches the image returned by getImage() which breaks the contract of ImageDescriptor and complicates life-cycle management of the image. From the documentation of ImageDescriptor: "It does not hold onto images or cache them, but rather just creates them on demand."

Comment 1 Steffen Pingel

2009-12-09 23:27:16 EST

Created attachment 154188 [details]
committed patch

Comment 2 Steffen Pingel

2009-12-09 23:27:18 EST

Created attachment 154189 [details]
mylyn/context/zip

Comment 3 Steffen Pingel

2009-12-09 23:32:47 EST

Created attachment 154190 [details]
proposed patch

Comment 4 Steffen Pingel

2009-12-09 23:38:29 EST

Mik, the highlighter implementation leaks colors in many places throughout the code. While it is possible to fix this I estimate that it would at least take a few hours. I am not sure that it's worth investing time into this but on the other hand I don't believe we can afford supporting this feature in the current state. 

I have attached a patch that removes the highlighter feature. Let me know if you are okay with applying that. If we have slack in the future we can always go back, revert the patch and work on a fix.

Comment 5 Jörg Thönnes

2009-12-11 06:12:12 EST

I do not use the highlighter feature, since I never fully understood how I could benefit from it.

+1 for removal.

Comment 6 Frank Becker

2009-12-16 16:09:40 EST

+1 for removal.

If someone need this we can implement highlighter for the tasklist
 after we have solved Bug# 199345.

Comment 7 Steffen Pingel

2009-12-19 22:19:13 EST

Thanks for the feedback. I'll go ahead with removing the feature in January then.

Comment 8 Steffen Pingel

2010-02-03 15:49:37 EST

Created attachment 158097 [details]
updated patch

Comment 9 Steffen Pingel

2010-02-03 15:50:22 EST

Committed patch which removes the highlighter functionality.