Bug 160572 - [new eux] add ability to attach images to tasks
Summary: [new eux] add ability to attach images to tasks
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P2 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Balazs Brinkus CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 182967 (view as bug list)
Depends on: 78856
Blocks: 195689
  Show dependency tree
 
Reported: 2006-10-11 17:59 EDT by Ken Geis CLA
Modified: 2008-10-01 00:50 EDT (History)
3 users (show)

See Also:


Attachments
screenshot page (17.21 KB, patch)
2007-06-25 09:44 EDT, Balazs Brinkus CLA
no flags Details | Diff
mylyn/context/zip (4.27 KB, application/octet-stream)
2007-06-25 09:45 EDT, Balazs Brinkus CLA
no flags Details
screenshot page v2 (15.24 KB, patch)
2007-06-28 10:08 EDT, Balazs Brinkus CLA
no flags Details | Diff
region screenshot (7.50 KB, patch)
2007-07-29 11:28 EDT, Balazs Brinkus CLA
no flags Details | Diff
crop picture (9.50 KB, patch)
2007-07-31 20:55 EDT, Balazs Brinkus CLA
no flags Details | Diff
crop picture aspect (11.16 KB, patch)
2007-07-31 20:56 EDT, Balazs Brinkus CLA
no flags Details | Diff
crop picture v2 (13.22 KB, patch)
2007-08-06 15:28 EDT, Balazs Brinkus CLA
no flags Details | Diff
screenshotpage icons (1.26 KB, application/octet-stream)
2007-08-06 15:29 EDT, Balazs Brinkus CLA
no flags Details
crop picture v2 with revert (14.33 KB, patch)
2007-08-06 20:28 EDT, Balazs Brinkus CLA
no flags Details | Diff
screenshotpage icons v2 (1.54 KB, application/octet-stream)
2007-08-06 20:32 EDT, Balazs Brinkus CLA
no flags Details
crop same window (14.44 KB, patch)
2007-08-12 18:05 EDT, Balazs Brinkus CLA
no flags Details | Diff
crop ui (89.44 KB, image/png)
2007-08-14 12:50 EDT, Robert Elves CLA
no flags Details
capture (14.43 KB, patch)
2007-08-17 18:22 EDT, Balazs Brinkus CLA
no flags Details | Diff
capture icons (1003 bytes, application/octet-stream)
2007-08-17 18:22 EDT, Balazs Brinkus CLA
no flags Details
mylyn/context/zip (13.83 KB, application/octet-stream)
2007-08-28 00:45 EDT, Mik Kersten CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ken Geis CLA 2006-10-11 17:59:10 EDT
When I go to add an attachment, I'm greeted with the text "Clipboard contents are for text attachments only."  However, when I get to the next page in the "add attachment" wizard, I'm allowed to select the Content Type, including various image mime types.

The behavior I would want is like this.  If my clipboard has text in it, I click Next and then I can select from text/plain, text/html, or application/xml for the Content Type.  If there is an image in my clipboard, I can click Next and then select from image/gif, image/jpeg, or image/png as the Content Type.  The plugin would convert from the clipboard image to whichever Content Type is selected.
Comment 1 Eugene Kuleshov CLA 2006-10-11 20:44:23 EDT
The problem is that SWT does not provide platform independent API for retrieving images from clipboard. I think Brock wrote some code that works on Windows...
Comment 2 Ken Geis CLA 2006-10-11 20:54:57 EDT
I added bug 78856 as a dependency.

In any case, and maybe this should be logged as a different bug, the UI should prevent you from creating a text attachment from the clipboard and marking it with a Content Type of image/*.
Comment 3 Eugene Kuleshov CLA 2007-04-18 11:49:41 EDT
*** Bug 182967 has been marked as a duplicate of this bug. ***
Comment 4 Mik Kersten CLA 2007-04-20 09:49:31 EDT
It would be nice to have this feature, since it is a pain for people to have to use an external program just to generate an image.

Brock: do you still have this code?  I assume that you had it working for Windows only?

Eugene also linked the following related snippet: http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet215.java?view=co
Comment 5 Eugene Kuleshov CLA 2007-04-20 10:31:17 EDT
Mik, I think that code is attached to the bug 78856 as well as the prototype for other platforms that didn't made to 3.3
Comment 6 Mik Kersten CLA 2007-04-23 08:44:54 EDT
Tagging this as a "new user experience" feature because new users need an easy way to provide feedback on UI problems that they see and the ability to quickly attach a screenshot to a bug report would assist that.
Comment 7 Balazs Brinkus CLA 2007-06-25 09:44:59 EDT
Created attachment 72353 [details]
screenshot page

I finished the screenshot function. This isn't a final version, I want to fix some things.
I have attached this patch that you can test. I tested it only in Windows. In the evening I will try on Linux.
Comment 8 Balazs Brinkus CLA 2007-06-25 09:45:06 EDT
Created attachment 72354 [details]
mylyn/context/zip
Comment 9 Eugene Kuleshov CLA 2007-06-25 11:15:06 EDT
Balazs, it seems like you aren't using most recent changes from CVS. Patch look stale.
Comment 10 Balazs Brinkus CLA 2007-06-28 10:08:48 EDT
Created attachment 72683 [details]
screenshot page v2

Here is the new version of the patch. I used the latest code from the CVS.
Comment 11 Mik Kersten CLA 2007-06-29 14:08:31 EDT
Thanks Balazs.  Will review next Tuesday.
Comment 12 Mik Kersten CLA 2007-07-06 14:49:49 EDT
Patch applied.  Excellent work, this will be a *very* useful feature for helping users point out problems.  I now think that it is worth investing a bit more time in enahncing this functionality, so I created bug 195689 and bug 195691.
Comment 13 Eugene Kuleshov CLA 2007-07-06 15:06:03 EDT
I am concerned. Current implementation takes full screen screenshot instead of the particular window (or even just a selected view or editor). It also forces editor where screenshot will be attached into the focus. Both of those restrictions significantly limiting use of this feature.

Possible alternative could be allow configurable delay before screenshot is taken to allow user to hide or bring up required parts. 

Another option is have a toolbar button that would take screenshot and stick it into the clipboard. It could use proprietary transfer object and don't have to wait for image transfer in SWT, so "clipboard" option on attach dialog would be able to recognize that image.
Comment 14 Mik Kersten CLA 2007-07-06 16:43:54 EDT
I think the configurable delay is way too complex.  The typical way that tools deal with this is to allow you to select the window/area to be captured, or to allow you to edit the screenshot afterwards (disucssion about that should go on bug 195689).  I was tempted to file a bug for clipboard-based capture since that would be my preferred way of doing it (works great on Windows) but that's an advanced UI too and many advanced users already know how to create and capture screenshots.

Balazs: is it easy for you to turn the clipboard contents into a captured image if they are a bitmap, or is that OS-specific behavior?
Comment 15 Eugene Kuleshov CLA 2007-07-06 17:01:27 EDT
(In reply to comment #14)
> I think the configurable delay is way too complex.  

I don't think it is and it is often the only option if you need to capture menus, popups or tooltips

> I was tempted to file a bug for clipboard-based capture since that
> would be my preferred way of doing it (works great on Windows) but that's an
> advanced UI too and many advanced users already know how to create and capture
> screenshots.

Well, advanced users also can fill bugs trough the web interface, but we should make things quick and easy even for those users.

> Balazs: is it easy for you to turn the clipboard contents into a captured image
> if they are a bitmap, or is that OS-specific behavior?

Reading clipboard populated by PrtScreen or Alt-PrtScreen would require bug 78856. Though that bug has a patch that could allow to handle that on several popular platforms. That is why I suggested to use proprietary transfer object (that is simple to implement and it won't be OS-specific), using it "capture" action would use to store image into clipboard and attachment dialog will be able to read it back.
Comment 16 Balazs Brinkus CLA 2007-07-29 11:28:00 EDT
Created attachment 74874 [details]
region screenshot

I implemented a function which helps to make a screenshot from an area of the screen.
It contains few problems yet (e.g: GC draw), so it's only a preview. 
I'm insteristed in your opinion about this patch.
Comment 17 Eugene Kuleshov CLA 2007-07-29 12:37:24 EDT
It sort of works, but there is way too many glitches and artifacts. I'd suggest to take a full window screenshot and then allow to crop it.

Also, picture shown on the screenshot capturing page is resized to fit window (imho, that is an arguable feature) and the worst part that it doesn't preserve the aspect ratio.
Comment 18 Robert Elves CLA 2007-07-30 01:03:51 EDT
 (In reply to comment #16)
> It contains few problems yet (e.g: GC draw), so it's only a preview.
> I'm insteristed in your opinion about this patch.
I managed to move a file within the Package Explorer while trying to select the screenshot region. Eugene may have a point - perhaps taking a screenshot and allowing the user to crop would be easier and less error prone?
Comment 19 Balazs Brinkus CLA 2007-07-31 20:55:04 EDT
Created attachment 75088 [details]
crop picture

I added a button which opens a dialog where you can modify the screenshot. 
I made 2 different versions. The first one (crop picture) is build on the original version, so it fits the image on the canvas. 
In the other version (crop picture aspect) I implemented a scrollcomposite, so the picture preserves the original size.
Sorry because I'm late, but my computer had a hardware problem, but it's working now.
Comment 20 Balazs Brinkus CLA 2007-07-31 20:56:27 EDT
Created attachment 75089 [details]
crop picture aspect
Comment 21 Robert Elves CLA 2007-08-01 19:14:46 EDT
Sorry for delay Balazs, I will be able to look at this tomorrow.
Comment 22 Robert Elves CLA 2007-08-02 12:45:35 EDT
This is getting very slick Balazs. I think your first patch where you're scaling the screenshot to fit within the wizzard dialog is best. Less scroll bars the better!  Would it be possible to do the crop selection in place vs opening the additional window?  I can envision the following buttons on the create screenshot page:

[Capture] [Crop] [Revert]

User presses [Capture] and screenshot is shown as you have now.  Upon capture a message could appear above buttons (perhaps in wizard message area) stating "Drag selection on image to crop". Once user makes selection the [Crop] button becomes enabled. User presses [Crop] and cropped image is displayed. User can then crop again if they want. Upon pressing [Revert] (enabled after a crop has been performed) the original screenshot is restored in place.

Is this feasible?
Comment 23 Eugene Kuleshov CLA 2007-08-02 13:12:00 EDT
Can we have an option to fit image to window (preserving the aspect ratio) or show it on the scroll panel?

Rob, we don't really need "Crop" and "Revert" buttons (less UI is better) and it will also eliminate issues like "cropped too much" that would usually require undo action. So, selected area should be picked up automatically and user should be able to change selection and then user could see result on the last wizard page (preview), which should be restored for the screenshot attachment.

Reopened issue, since we still have upcoming patches.
Comment 24 Balazs Brinkus CLA 2007-08-06 15:28:24 EDT
Created attachment 75449 [details]
crop picture v2

I made a new version. It contains a button which helps to switch between the "fit image" and the "aspect ratio keep" lookout.
This version does not contain the revert function, but I will attach this function too, then you can compare the two solution.
I added icons to the buttons (I will attach the icons as well). What do you think about this?
Comment 25 Balazs Brinkus CLA 2007-08-06 15:29:27 EDT
Created attachment 75450 [details]
screenshotpage icons
Comment 26 Balazs Brinkus CLA 2007-08-06 20:28:56 EDT
Created attachment 75466 [details]
crop picture v2 with revert

I made a version which contains the revert function too.
Comment 27 Balazs Brinkus CLA 2007-08-06 20:32:23 EDT
Created attachment 75467 [details]
screenshotpage icons v2 

New version of icons. It contains the revert button icon too.
Comment 28 Robert Elves CLA 2007-08-08 12:45:08 EDT
Nice! It's great to have the option of fit vs 1:1 right within the same dialog. Have you given any thought to the notion of performing the crop within the same page? The buttons/icons are a good start and Mik will want to iterate on these. Also (as Eugene mentioned) less ui the better so if we can allow selection of the region to crop and just gray out what isn't contained within the cropped region this would eliminate the need for the crop button. Thoughts?
Comment 29 Balazs Brinkus CLA 2007-08-09 20:22:09 EDT
> Have you given any thought to the notion of performing the crop within the same page?
Yes. I tried it, but I don't like it. I have some problem with the GC. 
E.g: you select the section which you like to crop and you have the crop region border too. So if you make some modification on the page (scroll or change page with alt+tab) the region border will disappear.
>  Also (as Eugene mentioned) less ui the better so if we can allow selection of the region to crop and just gray out what isn't contained within the cropped region this would eliminate the need for the crop button. 
Yes. It's a little bit crowded with this button. So I thought to move this button to the crop window. First when I tried to implement it, I put it to the crop window, but I had some refresh problem, then I moved it to the screenshot page and there it worked correctly, so I kept it.
For the gray coloring solution: I thought that too when I started it, but I did not start to implement it. I will try to implement this.
Comment 30 Eugene Kuleshov CLA 2007-08-09 21:20:22 EDT
Buttons are ok, but we really need to figure out how to crop within the same window...
Comment 31 Balazs Brinkus CLA 2007-08-12 18:05:27 EDT
Created attachment 75931 [details]
crop same window

I made a new version. You can crop in the same window. The regions which are out of the crop region will be grayed out.
Comment 32 Robert Elves CLA 2007-08-14 01:54:38 EDT
Excellent Balazs!! Very very cool.  Now we just need to move the button images so they are managed by TasksUiImages so we don't have any leaks and consider alternative button layout/interactions which we can discuss on the conference call tomorrow! This is really impressive and I can see this getting used a lot!
Comment 33 Robert Elves CLA 2007-08-14 12:50:04 EDT
Created attachment 76061 [details]
crop ui
Comment 34 Robert Elves CLA 2007-08-14 14:41:24 EDT
As per conference call lets update the fit image button to be a check box style button, change the Capture full screen button to simply read "Capture" and consider removing the crop button which is practically unnecessary but if we remove it we'll require some affordence to indicate the ability to crop by selection, perhaps changing the cursor when over the image?
Comment 35 Eugene Kuleshov CLA 2007-08-14 17:46:25 EDT
btw, had anyone thought about how to capture screenshots of the non Eclipse windows?
Comment 36 Ken Geis CLA 2007-08-14 18:14:22 EDT
(In reply to comment #35)
> btw, had anyone thought about how to capture screenshots of the non Eclipse
> windows?

I've been following this bug, but I haven't been trying out the work in progress.  I'm excited to see what you've come up with, but if it cannot capture screenshots outside of Eclipse, it does not handle my use case.

I would prefer that the effort had gone into fixing bug 78856 instead of developing a new user interface to solve a problem that I didn't have.  I have no trouble collecting a screenshot.  The existing user interface was completely intuitive, but the underlying functionality was missing.

I hope that both ways of getting a screenshot into a task will be available in the Eclipse 3.4 timeframe, when bug 78856 is hopefully resolved.
Comment 37 Balazs Brinkus CLA 2007-08-15 20:10:06 EDT
I tested the capture function on Ubuntu Linux 6.10 (I downloaded a vmware image). It worked there correctly.
I will test it on Max Os X too (I started the downloading of the image file 2 days ago).
Rob: I will make the changes. I have some idea for the crop function.
Comment 38 Balazs Brinkus CLA 2007-08-17 18:22:12 EDT
Created attachment 76356 [details]
capture

I made a new version. I wil attach the icons too (I renamed the icons).
Comment 39 Balazs Brinkus CLA 2007-08-17 18:22:59 EDT
Created attachment 76357 [details]
capture icons
Comment 40 Mik Kersten CLA 2007-08-21 11:56:45 EDT
 (In reply to comment #36)
> I would prefer that the effort had gone into fixing bug 78856 instead of
> developing a new user interface to solve a problem that I didn't have.  I have
> no trouble collecting a screenshot.  The existing user interface was completely
> intuitive, but the underlying functionality was missing.
> 
> I hope that both ways of getting a screenshot into a task will be available in
> the Eclipse 3.4 timeframe, when bug 78856 is hopefully resolved.

Ken: I agree that it would be great to have bug 78856 fixed, and voted for that bug quite a while ago.  However, that bug involves low-level SWT and OS internals, and is not something that in scope for the Mylyn project.  Balazs is doing a great job of meeting the use case of allow users to capture screenshots of Eclipse to improve the feedback loop.  I realize that we have forked some from your original description of this bug is more generic that we will be able to do without a fix for bug 78856, so we should leave this one open until that's done, and I have created bug 200706 to track additional discussion not specific to the more generic support.  In the meantime, I suggest that you post a comment on bug 78856 asking if it is being considered for Eclipse 3.4.  You can also mention that Mylyn has gone as far as we can in supporting your request but cannot support generic clipboard capture without that fix.
Comment 41 Mik Kersten CLA 2007-08-21 12:00:41 EDT
Balazs: before Ken posts, could you summarize for us the limitations of your current implementation in terms of capturing non-Eclipse windows?
Comment 42 Mik Kersten CLA 2007-08-28 00:44:59 EDT
Remaining patch applied.  I had to make a fix to prevent invalid thread access due to cropping, and used more standard images for the icons (one from WTP one from the Platform).  Please look over the changes and make sure that all is to your liking.  Closing bug 200706.

This is fantastic stuff Balazs, and will be in the 2.1M1 New & Noteworthy.

Regarding non-Eclipse windows, Balazs' current patch will allow you to capture those Windows as long as they are visible (verified on Vista only).  You just need to crop the capture accordingly.  If this is still lacking desicred functionality please reopen.
Comment 43 Mik Kersten CLA 2007-08-28 00:45:06 EDT
Created attachment 77091 [details]
mylyn/context/zip