Community
Participate
Working Groups
These are used very frequently. Drag-and drop attaching would be nice.
Current work-around is to open the report in the internal browser.
also a hyperlink to view attchments in the Bug View would be nice :)
Could you make a new report for that, it seems quite easy to do. I've clarified the description of this bug.
BTW, it would be also handy to allow to create attachment from a clipboard. Eg. when attaching patch created by one of the source control providers...
Eugene commented on bug 143017: Can we actually open attachments in Eclipse editor if content type is known? And also have "Save As..." button or something like that.
What is the status of this feature? I noticed BugzillaRepositoryUtil::uploadAttachemt(), is that complete and just the UI bits are missing or is there more back-end work to be done? Also Rob, Mik mentioned you have tests for this as well, where would I find them?
Jeff, yes the upload/download attachment is in use for attaching contexts to bug reports. Currently there is no UI for uploading generic attachments although the functionality is there. The methods may need to be upgraded to handle content types properly but other than that it should all be there. I did a quick look for the unit tests myself and couldn't locate them. They were commented out so may have gotten cleaned by accident.
This feature is in progress but I don't have permission to change the asignee. If someone wants to assign it to me that would be dandy.
Great! Re-assigned to Jeff, removed helpwanted.
So the first step for this is straighforward: adding a file chooser to the bug editor, and having that use the uploadAttachment functionality. But in terms of UI, the two options seem to be: 1) Have a "new attachment" field that will display the path, and submitting will result in the attachment being added. 2) Have an Add button on the table. If so, it may be best for Add to launch a wizard dialog. What do you think Jeff?
I have a prototype in the works right now that adds a button in the attachments section and displays the selected file path beside it. Upon selecting an attachment, a field for the attachments description and attachment comment are added. The attachment is uploaded by submitting the bug through the editors regular submit button (it would be easy enough to have a separate button for that though) I just need to add support for choosing/detecting the content type and I'll post a patch for opinions.
Sounds good Jeff. It will be helpful if you attach a task context along with the patch. Also, remember to synch frequently and use the Change Set support to making seeing our changes and patching easier.
Created attachment 43048 [details] Add attachments to bugzilla reports Here's the first cut. The downsides thus far are: doesn't allow user to pick content type but auto-detects more common file types. Also, SWT allocates blank space for invisible widgets, maybe we could show them but leave them disabled instead of hidding them. Also, it doesn't warn the user that a description is required. Uploading will fail without one. I'm uploading this patch from Mylar (crosses fingers) ;)
I just reviewed the patch, good start Jeff. First thing to do next is write a test case for this, added to BugzillaRepositoryConnectorTest or to a new test. You can look to other Bugzilla tests as an example, and the general idea should be to attach a file to an exisitng report, then synch the report and check that there is one more attachemnt than before. They can be tricky to write so holler if you have questions. But the tests are extemely valuable. For example, uploading a 0K .txt file failed silently for me. Once you have the test harness for this in place it will be easy to add new test cases like that (not sure we need one for that corner case).
How's it going Jeff? One thing I forgot to mention is that RepositoryReportFactoryTest.testReadingReport() has some commented out code for checking attachments that you could work on. One problem that you'll likely run into is providing credentials. As the first pass you an put your test account as strings in the code, but to avoid potential trouble I think we should probably read these from a local properties file that is not checked in.
Things are going well, so far I've just been adding code to BugzillaRepositoryConnectorTest, Ill have a look at RepositoryReportFactoryTest.testReadingReport(). Using a local properties file is probably a good idea, or even just having a simple test account for all tests to use might work. One thing I've realized is that I'll have to create a new report to test attachments since using the same report could cause a race condition if multiple people run the unit tests at the same time. I suppose its not a big deal if the test bugzilla starts accumulating a lot of bugs (and attachments). If its a problem we could always use landfill.bugzilla.org.
I would suggest to put such properties under user home dir ~user or "docuements and settings/user" on windows...
hrm, updating today broke my workspace pretty good. What happend to IBugzillaBug? Files changed in my workspace (and my patch) have been moved or changed in HEAD. I'll have to reimplement some of the patch to get compatiability back. Should I include test cases in the next patch to get it comitted faster, or do you want them as separate patches?
Apologies Jeff, we're in the midst of some refactoring there. If you have a patch today we can try to hold off changing more class names for the next few hours until we get your patch integrated. IBugzillaBug is gone, use the RepositoryReport class instead. Other than that the internals haven't changed much so you should be OK. Regarding responsiveness applying patches, I will usually be able to do that within a 1-day turn-around for you if I can apply them on the first go. A test case always helps ensure that. Yes, you can always create a new bug report, just prefix it's description with the test name or something like that. That server is meant to be our landfill.
Mik, go ahead and continue with the refactoring today. I'll keep going with the JUnit tests (I don't really need to be able to run them to come up with tests atm) and I'll adjust my work to your refactoring over the weekend.
Sounds good. Will keep you posted.
Jeff, this refactoring is very signficiant and we've put it in a branch. If you make your patch this weekend one of is should be able to merge it (maybe with some help). Fyi the refactoring is bug 144998.
Created attachment 43423 [details] Add attachments to bugzilla reports Modified to patch cleanly against HEAD, 2006/06/04. Includes some JUnit testing.
Thanks Jeff, I'll merge this in by end-of-day Monday after we merge our branch back into head.
Sounds good Mik, let me know when you've finished merging the branch and I'll have a look to see what changes need to be done to the patch. I can revise it or help you with merging it.
Good stuff Jeff, patch applied (Rob and I have merged the branch back). Now we're ready for the UI refinements. How about this: - make the button called "Add...", and put it on the right-hand side of the table. This will make it consistent with the plugin.xml editor which is good to use as an exmaple. - the button brings up a single-page Eclipse wizard that allows the description, summary, and content type to be edited. That page has a "Browse" button and field with the full path to the file (later we can make it smarter about creating patches, screenshots, etc, via additional pages) - make the result of the add created a new table entry that gets sorted to the last position, but whose creation date is "<not yet submitted>"
One more thing: it would be great to have a right-click action on that table for "Save As" so that we could save attachments. Also note that tomorrow Rob and I will be doing some refactoring of the editor to bring it up into mylar.tasklist, but will make sure to keep you posted and not change any implmenetation so that a merge is easy if needed.
Mik, I like the idea of adding an uncommitted attachment to the table. I'll look into that and right-click saving attachments. We should also consider saving uncommitted attachments as part of drafts.
Great. Btw, with the new common offline storage chances are that the paths to the attachments will already be saved before submit, and yup, that will be nice to have. Rob will soon be committing the move of the common editor stuff from bugzilla to tasklist. If you've got a patch now go ahead and submit it, otherwise let us know if you have any trouble mapping it to the moved classes.
(In reply to comment #29) > Great. Btw, with the new common offline storage chances are that the paths to > the attachments will already be saved before submit, and yup, that will be nice > to have. Hey, how about attaching right from the clipboard? Would be really handy for both, patches and screenshots...
The screenshot case would be tough because we would need to write the contents of the clipboard (BMP on Windows I think) into a file. But Brock has a screenshot tool he was thinking of contributing so I'm CC'ing him. But on that note, Jeff, you could make the wizard page pretty much identical to Team -> Apply Patch, which allows the contents to come from Clipboard, File or workspace.
(In reply to comment #31) > The screenshot case would be tough because we would need to write the contents > of the clipboard (BMP on Windows I think) into a file. That was exactly my point. Why would you need file for something stored in a clipboard (e.g. from Alt-PrintScreen on windows)? In a worst case you can create a temporary one or use Mylar's repository area. > But Brock has a screenshot tool he was thinking of contributing so I'm CC'ing him. Not sure about SWT/JFace, but it is fairly simple with AWT/Swing...
For clipboard image support see bug 78856 (should be avilable in 3.3) The screenshot tool was more than just capturing the image. It allows you to manipulate and annotate the image before attaching to the issue.
(In reply to comment #33) > For clipboard image support see bug 78856 (should be avilable in 3.3) Brock, can your image transfer be used without changes in SWT? So, it will at least work on windows. > The screenshot tool was more than just capturing the image. It allows you to > manipulate and annotate the image before attaching to the issue. Sure, that can be a good addition for those who don't have editors and don't mind to attach 4Mb screenshots with true color palette. ;-)
(In reply to comment #34) > Brock, can your image transfer be used without changes in SWT? So, it will at > least work on windows. No SWT changes required, but it only worked in windows. You would be able to copy out the new ImageTransfer classes from bug 78856, so you don't really need to wait for 3.3 (so long as you don't mind having org.eclipse.swt packages) > > Sure, that can be a good addition for those who don't have editors and don't > mind to attach 4Mb screenshots with true color palette. ;-) Not dure i follow you here. The image is converted to jpg before sending to the server, so the image is quite small. PNG would be a better option, but there is no write support for PNG in SWT yet.
(In reply to comment #35) > > Sure, that can be a good addition for those who don't have editors and don't > > mind to attach 4Mb screenshots with true color palette. ;-) > > Not dure i follow you here. The image is converted to jpg before sending to > the server, so the image is quite small. PNG would be a better option, but > there is no write support for PNG in SWT yet. Compare size and quality with GIF with 8 to 64 colors palette with no diffusion...
Jeff, please note that ".psd" attachments are currently failing, presumably because there is no way to set their content type set to binary.
Jeff, what's your ETA for the items on comment#26? I'm wondering if it will make to 0.5.3, which we will have a dev build of today and release on Monday.
Jeff, as I mentioned on comment#5 we should not keep valid credentials checked into CVS, so I updated BugzillaRepositoryConnectorTest to retrieve the credentials from a credentials.properties file. Instructions are in: http://www.eclipse.org/mylar/doc/contributing.php#contributing-setup
Mik, ive (theoretically) done most of the implementation for the wizard but I can't currently test it because HEAD does not work for me at the moment. I get empty mylar views with no repository connectors installed. I imagine my workspace is just stuck in the middle of some refactoring. I'll update and try it this weekend.
HEAD should always be working and all tests passing, so always make sure to scream at us if it isn't. Note that sometimes you'll need to do a full clean to get things working, and if your workspace gets messed you may also need to create a new runtime configuration.
Created attachment 44555 [details] Adds a new attachment wizard Here's a preliminary patch for the add attachment wizard. It currently only works for files on disk, clipboard and workspace resources are in progress (though the UI is there). After an attachment is selected, a text field under the attachments table shows the not yet submitted attachment. I've been having some issues trying to add it as a row in the table. Mik, Im posting this patch in case you want the wizard to go into tomorrows dev build. I'll post an updated patch with support for clipboard and workspace resources later (likely not in time to be reviewed for tomorrows dev build though)
Created attachment 44559 [details] mylar/context/zip attaching context for apply patch wizard
Apply patch wizard still needs support for obsoleting attachments.
Very nice! This definitely feels like the UI and you've kept it clean. Patch applied and manually tested with a filesystem file. In general we try not to add stuff to the UI if it doesn't work in order to avoid bug reports on dev builds or going out with partial functionality, so it would have been better to either disable that radio button or not have it there. There's still some finessing here (e.g. wizard description, etc) but in general that's always fine to leave up to me since I try to keep it consistent for all of Mylar. This will go out with tonight's dev build, and it looks like all we need to close this report is clipboard and workspace resources (which should have probably been separate bugs).
Created attachment 44700 [details] Add attachment from file system, workspace, or clipboard Revised patch now supports attaching workspace resources and *text* from the clipboard.
Couldn't apply the patch due to conflicts. Could you synch and try again?
Created attachment 44715 [details] Add attachment from file system, workspace, or clipboard Ah, my local copy of the new files weren't under version control and conflicted in the update. New patch should apply cleanly.
Patch applied and looking good. Got this exception when canceling out, please fix: java.lang.NullPointerException at java.io.File.<init>(Unknown Source) at org.eclipse.mylar.internal.tasklist.ui.editors.AbstractRepositoryTaskEditor$10.widgetSelected(AbstractRepositoryTaskEditor.java:843) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:90) Also, the wizard will take the full height of the window on a large workspace. Please limit it's height to 800 or so. Regarding patches: 1) add an @author tag to each class that you create 2) hit Ctrl+Shift+F to auto format the current file before creating the patch 3) what's going on with InputAttachmentSourcePage, is it a copy of another Eclipse class that couldn't be extended? If so make this very explicit by putting Copy in the type name and pointing to the original class. In general we have to avoid all copied code, although sometimes we can make exceptions from applicable classes in the Platform, as could be the case here.
Another nit: the "Add..." button ends up in the wrong position (far right) if there are no attachments on a report.
>what's going on with InputAttachmentSourcePage, is it a copy of another >Eclipse class that couldn't be extended? Yes, its a modified verion of internal class InputPatchPage from the apply patch wizard (org.eclipse.compare.internal.patch). I made a note of that at the top but perhaps it wasn't explicit enough. I'll make the fixes you've pointed out and post up another patch sometime tomorrow.
Created attachment 44897 [details] Fixes for comments 49 & 50 Attaching a patch addressing the issues in comment 49 and comment 50. Mik, I'm not sure what you mean by "putting Copy in the type name", I put a clearer comment at the top of the file. If you want something more particular I can make another change.
InputAttachmentSourcePage is fine now that you've updated the copyright header. What I was referring to is that if I create an exact copy of class Foo from Platform (e.g. to open up accessibility) I will call it FooCopy to indicate this, make a bug report for Platform, and not let it evolve independently. In this case it is fine for the new class to evolve independently to meet your needs. I applied the patch and the too-tall wizard problem is still there. Other than that nit is there anything else left before we can mark this RESOLVED?
I just tried to attach a screenshot but no path appeared after adding the wizard. Tried with both workspace resource and external file. Let me know if it is working for you in case I need to provide additional steps to reproduce.
Created attachment 44970 [details] fixes comment 54 Mik, my last patch didn't check for cancelation properly. This patch fixes that, all types of attachemnts should work now.
re: the height issue.. I though setting the heightHint would fix that, I'll look into another solution. Right now, the behaviour would be the same as the apply patch wizard. We could always file a bug with them (and probably should even if we come up with a solution).
Great, patch applied and attachments working now. The Apply Patch wizard does not max out the height on my current display (1200 tall), perhaps because of the way that it remembers the shell preferences (or those returing some max value). The easiest thing is probably to ensure the correct size/settings when you create the NewAttachmentWizard. This is worth doing because most people have a large workspace, and when we last put out a wizard like this a few complaints came in ;)
Created attachment 45210 [details] Screenshot of attachment wizard on Win XP This doesn't look right. Also, first page of the wizard is maxed out vertically. It seems vertical hint is not set for the tree widget up there. I am running latest dev build.
While I didn't test them without the XP manifest (as in your screenshot), Jeff's latest changes are only in HEAD. A new dev build should be available this evening, but there's a chance it may have to wait until Sunday.
(In reply to comment #59) > While I didn't test them without the XP manifest (as in your screenshot), > Jeff's latest changes are only in HEAD. A new dev build should be available > this evening, but there's a chance it may have to wait until Sunday. It is exactly the same UI in the HEAD. The problem there is that for some reason this single page is using FormToolkit instead of plain SWT UI like on 1st wizard page. However the good new is that SWT Designer can easily open both pages, so it won't take long time to redo the UI in it.
Jeff, this would be a good polish item for tnext week and after this and the height is done we should be able to mark this report as resolved.
I'll fix the height problem. Im not sure I'll be able to fix the layout issues Eugene is seeing though, everything looks fine on OSX so I can't really see the effect of changes. I could remove use of the toolkit as a start.
Created attachment 45245 [details] Fix max vertical space Here's a fix for the height of the dialog. I tried using heightHints but I couldn't get it to work so i just explicitly set the size of the shell. As a result, I have to set the location of the shell (it gets reset after the size is changed). If anyone has a suggestion on how to do this without going straight to the shell I'm all ears :) >The Apply Patch wizard does not max out the height on my current display Thats interesting, it maxes out on my display (OSX, 1440 x 900). Maybe the min is just larger than what I have.
Hmmm, the parent offsets could make it feel inconsistent with other workbench wizards. Isn't the right way to do this with dialog settings, as the patch wizard does? static class PatchWizardDialog extends WizardDialog { private static final String PATCH_WIZARD_SETTINGS_SECTION = "PatchWizard"; //$NON-NLS-1$ PatchWizardDialog(Shell parent, IWizard wizard) { super(parent, wizard); setShellStyle(getShellStyle() | SWT.RESIZE); setMinimumPageSize(700, 500); } protected IDialogSettings getDialogBoundsSettings() { IDialogSettings settings = CompareUIPlugin.getDefault().getDialogSettings(); IDialogSettings section = settings.getSection(PATCH_WIZARD_SETTINGS_SECTION); if (section == null) { section = settings.addNewSection(PATCH_WIZARD_SETTINGS_SECTION); } return section; } }
Created attachment 45325 [details] Sets and persists wizard dialog size Thanks for the tip Mik, this patch sets the initial size to something reasonable and persists user changes to the dialog size.
Great, patch applied. Nice work Jeff, the attachment support is now very well-rounded and will be prominent in the 0.6 New & Noteworthy.
