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

Bug 383570

Summary:

[Progress] Progress view duplicates job name

Product:

[Eclipse Project] Platform

Reporter:

Markus Keller <markus.kell.r>

Component:

Assignee:

Markus Keller <markus.kell.r>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

daniel_megert, gheorghe, markus.kell.r, pwebster

Version:

3.8

Flags:

pwebster: review+
daniel_megert: review+

Target Milestone:

3.8.1

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Screenshot	none
Fix	daniel_megert: review+

Description Markus Keller

2012-06-26 13:06:26 EDT

3.8, got broken in M7 by bug 242144

The Progress view often renders the name of a job twice. Can be seen a lot with EGit's refresh jobs. Unfortunately, I initially thought this was a bug in EGit, but it turned out to be a bad fix for bug 242144.

To reproduce in the SDK, you can e.g. select type java.lang.String and then choose "Open Call Hierarchy".

Comment 1 Markus Keller

2012-06-26 13:07:26 EDT

This needs to be fixed in 3.8.1

Comment 2 Markus Keller

2012-06-26 13:16:10 EDT

Created attachment 217890 [details]
Screenshot

Comment 3 Paul Webster

2012-06-26 13:23:33 EDT

I won't get to this in 3.8.1, I'm already overcommitted.  We can move it to 3.8.2 when the milestone is created.


PW

Comment 4 Markus Keller

2012-07-02 15:39:40 EDT

Created attachment 218183 [details]
Fix

Phew, the ProgressInfoItem is really a disaster.

Here's what I found:
- the top label is often getJobNameAndStatus(..)
- the bottom label is
  a) one of: a clickable link, a label
  b) one of: getJobNameAndStatus(..), a result message, a "task: subtask" info

In the situation of comment 0, the bottom label was not visible in 3.7. The fix for bug 242144 in ProgressInfoItem#setLinkText(..) made the label and thereby also the duplication of getJobNameAndStatus(..) appear.


The best solution would be to dump the whole "design" and start afresh, this time with a strategy and some basic UI design knowledge.

For 3.8.1, the easiest solution is to detect the situation where the second label adds zero information and then just leave it empty. The patch implements this and is an ideal fit for the predominant coding style in that class. :/


To reproduce, you can e.g. set a breakpoint in jdt.ui's DeferredMethodWrapper#fetchDeferredChildren(..) line 79, or use EGit's "Synchronize with Workspace" feature.

Comment 5 Dani Megert

2012-07-04 08:05:17 EDT

Comment on attachment 218183 [details]
Fix

The fix is good. Personally, I would first check the taskString before calling #getData(String) as this is less expensive.

Comment 6 Markus Keller

2012-07-04 09:26:41 EDT

> Personally, I would first check the taskString before calling
> #getData(String) as this is less expensive.

I'm not sure about that. #getData(String) just performs a few simple comparisons, but getMainTitle() often performs string concatenation.
Anyway, both calls are negligible compared to other computations.

Comment 7 Paul Webster

2012-07-04 13:27:39 EDT

(In reply to comment #4)
> Created attachment 218183 [details]
> Fix


Thank you Markus.  I'm fine with the fix as-is.

PW

Comment 8 Markus Keller

2012-07-16 13:01:22 EDT

Released the patch to
R3_8_maintenance: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=883259041d69dec867e060fe32a471b392d1a595
master: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=b23e4756a7b0547ec36aea62e2b7574bd35b94ad

Comment 9 Dani Megert

2012-08-16 03:58:04 EDT

Verified in M20120815-1000 and M20120815-1200.