Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 344632 - Show attachment in task editor link opens web page when no attachment pressent
Summary: Show attachment in task editor link opens web page when no attachment pressent
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 minor (vote)
Target Milestone: 3.6   Edit
Assignee: Frank Becker CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-03 14:44 EDT by Sam Davis CLA
Modified: 2011-05-17 00:00 EDT (History)
3 users (show)

See Also:

Attachments
patch V1 (3.92 KB, patch)
2011-05-16 16:23 EDT, Frank Becker CLA 		no flags Details | Diff
mylyn/context/zip (21.92 KB, application/octet-stream)
2011-05-16 16:23 EDT, Frank Becker CLA 		no flags Details
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 Sam Davis CLA 2011-05-03 14:44:02 EDT 
Clicking Show attachment in task editor on a link to an attachment in a comment opens the web ui, if there is no attachment on that task (e.g. if the comment was pasted in from a different task). It should either do nothing or display an error message (or else not even be a hyperlink).
Comment 1 Steffen Pingel CLA 2011-05-06 16:12:59 EDT 
Frank, if it's not too expensive we could validate if attachment exists when parsing the text.
Comment 2 Frank Becker CLA 2011-05-08 02:33:12 EDT 
(In reply to comment #1)
> Frank, if it's not too expensive we could validate if attachment exists when
> parsing the text.

Steffen,

I see no way to check if the attachment is in the task.

BugzillaConectorUI.findHyperlinks has TaskRepository and ITask as parameters but not TaskData.

I see no way that we can get the needed TaskData in an cheap way.

Thoughts?
Comment 3 Steffen Pingel CLA 2011-05-08 05:23:57 EDT 
Thanks for checking. In this case we won't be able to suppress the hyperlink and should improve error handling instead.
Comment 4 Frank Becker CLA 2011-05-08 06:22:31 EDT 
(In reply to comment #3)
> Thanks for checking. In this case we won't be able to suppress the hyperlink and
> should improve error handling instead.

That we open the the attachment in the web page is the fallback.
Should we bring up an Dialog first so the the user can cancel the action?
Comment 5 Steffen Pingel CLA 2011-05-16 13:24:48 EDT 
Makes sense. I suggest something along the lines of "Attachment xyz is not attached to the open bug. Open attachment in web browser?"
Comment 6 Sam Davis CLA 2011-05-16 15:07:00 EDT 
We can't know what attachement to show in the web browser, because we don't know where the attachment link was pasted from.
Comment 7 Steffen Pingel CLA 2011-05-16 15:39:33 EDT 
Sam, can you post an example? Obviously we have to assume that the link is referring to the current repository unless it's a qualified URL. The same applies tasks or any other relative hyperlink but I'm not sure I understand the concern.
Comment 8 Sam Davis CLA 2011-05-16 15:53:23 EDT 
Nevermind, I forgot that the attachment link includes the unique id of the attachment, so opening in the browser works. Here is an example link:

(In reply to comment #6)
> Created attachment 195156 [details]
> framework patch
> 
> Here's the part of the patch for the framework.
Comment 9 Frank Becker CLA 2011-05-16 16:23:44 EDT 
Created attachment 195789 [details]
patch V1

can someone verify this so we can include this in 3.6?
Comment 10 Frank Becker CLA 2011-05-16 16:23:47 EDT 
Created attachment 195790 [details]
mylyn/context/zip
Comment 11 Steffen Pingel CLA 2011-05-16 16:30:31 EDT 
Looks good except for one nit, please replace the call to PlatformUI.getWorkbench().getDisplay().getActiveShell() by WorkbenchUtil.getShell().
Comment 12 Frank Becker CLA 2011-05-17 00:00:23 EDT 
patch V1 is now in HEAD with the changes requested in comment#11.