Community
Participate
Working Groups
doubleclick on repository attachment don't respect default workbench editor for selected content type. For example, I have quickimage plugin configured as default editor for gif, png and jpf files, but Mylyn still opens those files using web browser editor.
Attachments are opened in the web browser by design. Mik, what's your take on this? Should we change it the open action to use the default workbench editor and fall back to the web-browser if no editor is registered for the respective content-type?
Platform design allow plugins specify "default" editor for specific editor types, but it also allow users to assign their own editors as defaults. It would be really great to preserve this functionality.
That functionality should be available through "Open With Default Editor" from the context menu or does that not work as expected?
(In reply to comment #3) > That functionality should be available through "Open With Default Editor" from > the context menu or does that not work as expected? It is expected that double click would open default editor. I am aware of "open with default editor" action, which requires significantly more clicks.
My take is that the more that we can make the attachments feel like local resources the better, but we need to be careful because they are not local resources (e.g. retrieval might take a long amount of time or require authentication). Afaik the "Open" functionality in workbench is closely tied into the resource model, so this could be a considerable amount of work. +1 for considering it for post 3.0 planning.
I don't see how last comment is related to the open action. It is up to the editor how to handle resources and take care of long downloads. Mylyn can register web browser as default, but user should be able to override that in a same way as any other editors in workspace. This is just a standard workspace behavior designed to reduce number of mouse clicks required for open action. Also, in my knowledge of editor source API, editors are *not* bound to resources, so this comment is even more confusing.
For FogBugz connector which I am writing, it is very unfortunate that attachments are open in web browser by default. Problem is that FogBugz connector cannot provide correct URL for attachment, because URL should contain auth. token to be correct, and this token is obtained only after connector performs login. Token also changes between logins, so URL is not stable. Therefore I generate URL without this token, which MAY or MAY NOT work, depending on whether user uses IE (or other default browser) as his browser when viewing fogbugz repository or not. If he does, auth. token is not needed as it is sent in cookie by browser. I added custom 'Open in Default Program' command to attachment menu, which simply downloads attachment from remote repository and opens it in default program associated with attachment (not default editor). I would like to make this command as default for attachments from my connector, but it's not possible at the moment.
I have tentatively scheduled this bug for 3.2.
Peter, are you interested in providing a patch to change the default behavior?
(In reply to comment #9) > Peter, are you interested in providing a patch to change the default behavior? > Steffen, I will have some free time in about two weeks, and I can take a look at it then. Is there an agreement what default action should do? Should it simply use configured editor associated to the content type of the attachment, and browser as fallback?
(In reply to comment #10) > (In reply to comment #9) > > Peter, are you interested in providing a patch to change the default behavior? > > > > Steffen, I will have some free time in about two weeks, and I can take a look at > it then. Great! > Is there an agreement what default action should do? Should it simply use > configured editor associated to the content type of the attachment, and browser > as fallback? Yes, that sounds good to me.
Didn't Maarten Meijer already make some kind of patch for this?
I am not aware that there is a patch that changes the behavior of double click but please let me know if you have one. In case you are referring to bug 249021: The patch addressed a separate problem where an attachment does not have a URL.
Ah, okay. I figured that this code must be very closely related but that was just a guess.
Created attachment 146559 [details] first version of patch, unfinished I am attaching first version of patch. It works, but needs more polishing. I am attaching it to get some feedback. I tried to implement behaviour as suggested above, but ended up with something different. It mimicks behaviour of "Open" + "Open With" commands for standard resources. "Open With" can open attachment in browser, in-place editor, system editor or default text editor. Whichever is chosen, it is saved as 'preferred' viewer for Open command. "Open" simply uses most-preferred editor. "Open" is also default command when user double-clicks the attachment, so this effectively gives user an option to set his preferred way of viewing attachments. Attached patch also fixes problem with current implementation of TaskAttachmentInputEditor/TaskAttachmentStorage pair. TAStorage blocks UI thread when editor is fetching data for attachment. After some playing with these two classes, I believe that attachment must be downloaded _before_ we try to call page.openEditor(). Attached patch does that (in DownloadAndOpenTaskAttachmentJob). Attached patch is not finished. Remaining items: * improve and externalize strings, log reporting * get rid of TaskAttachmentInputEditor + TaskAttachmentStorage. There is one additional class using TaskAttachmentStorage -- Apply Patch action. I plan to rewrite it so that it downloads data on background thread, because currently it also blocks UI thread while downloading attachment. * remove old "Open in Browser" and "Open in Default Editor" commands and handlers -- is this OK? This can break existing clients using these commands :-( * testing on Linux and Mac
Created attachment 146560 [details] mylyn/context/zip
Created attachment 146579 [details] Second version with patch Changes since first version: * attachment viewer based on default eclipse editor is preferred over the browser (this was original point of this bug) * default attachment viewer (for double-click) is remember on per file extension basis * downloaded file is marked as read-only ... this gives user warning when he tries to modify/save temporary file in external editor * system-editor is always displayed in the menu. This is consistent with global Open With menu * Open With menu is populated with all editors based on filename (again, similar to global Open With menu) * cancelling attachment download (when opening in editor) now doesn't show error :-) * Removed class TaskAttachmentEditorInput and TaskAttachmentStorage, replaced with FileEditorInput + FileStorage, which use temporary file with downloaded attachment content. FileEditorInput + FileStorage are not responsible for downloading anymore. * Rewrote ApplyPatchAction to use new FileEditorInput + FileStorage. "Apply Patch" now downloads attachment first, and only then opens the Apply Patch wizard. It doesn't block UI while downloading patch anymore. * externalized strings Breaking changes: * Removed "org.eclipse.mylyn.tasks.ui.command.attachment.openInDefaultEditor" command from plugin.xml, and associated handler. * Removed "org.eclipse.mylyn.tasks.ui.command.attachment.openInBrowser" command and associated handler. Both commands are replaced with Open + Open With now. I believe the patch is finished now (modulo testing). I'll try to test it on Linux, but I don't have Mac for testing.
Created attachment 146580 [details] mylyn/context/zip
Created attachment 147047 [details] 3rd version of patch * Added Default Text Editor to the list of editors. * Fixed editor tooltip. Instead of using attachment URL, tooltip now says "Attachment from Bug 123456, RepositoryLabel".
Peter, the 3rd patch you supplied does not work with stream based attachments, see bug#249021 that is RESOLVED WONTFIX Also please see bug#249463 when the default editor is an external editor. For use cases for non URL based attachments see that bug and http://eclipsophy.blogspot.com and Eventum, where URL's have the form: http://server/eventum-2.2/download.php?cat=attachment&id=1 but come from blobs. In the data for stream based attachments the Industrial Connector can provide filename, mime type to the TaskAttachmentMapper. So maybe a heuristic can be devised that will make a best guess based on these when no URL is present and can handle a blob based attachment. I have submitted a TaskAttachmentPropertyTester patch some time ago, so you can test for "hasUrl" in the plugin.xml handler enablement. The Open With Browser handler should also include that test, but that is a separate bug I think.
(In reply to comment #20) > Peter, > the 3rd patch you supplied does not work with stream based attachments, see > bug#249021 that is RESOLVED WONTFIX > Also please see bug#249463 when the default editor is an external editor. Maarten, thank you very much for your feedback. My goal was to make opening attachments work even if there is no URL. This use case works for me. Would it be possible for you to put a breakpoint into TaskAttachmentViewersManager.getTaskAttachmentViewers(ITaskAttachment attachment) method, and see what 'viewers' (editors) are returned for your attachment? This method is called from new OpenTaskAttachmentWithMenu class, which fills "Open With" submenu with available viewers. All of the viewers except browser-based one use stream to download attachment to local file and open internal or external (system) editor on this temporary file. > I have submitted a TaskAttachmentPropertyTester patch some time ago, so you can > test for "hasUrl" in the plugin.xml handler enablement. > > The Open With Browser handler should also include that test, but that is a > separate bug I think. With my patch applied, "Open with Browser" should only be visible if attachment has an url. I also plan to get rid of attachment URLs since they cause problems in my connector too :-) so I had this goal in mind when working on this bug.
(In reply to comment #21) > My goal was to make opening attachments work even if there is no URL. This use > case works for me. > > Would it be possible for you to put a breakpoint into > TaskAttachmentViewersManager.getTaskAttachmentViewers(ITaskAttachment > attachment) method, and see what 'viewers' (editors) are returned for your > attachment? Does not trigger on Open With/Default Editor > With my patch applied, "Open with Browser" should only be visible if attachment > has an url. Gives the follwoing stacktrace on opening the browser window: org.eclipse.ui.PartInitException: Could not open Web browser on project-step21.png. Ensure that it is an uncompressed file. at org.eclipse.ui.internal.browser.WebBrowserEditor.init(WebBrowserEditor.java:229) at org.eclipse.ui.internal.EditorManager.createSite(EditorManager.java:798) at org.eclipse.ui.internal.EditorReference.createPartHelper(EditorReference.java:644) at org.eclipse.ui.internal.EditorReference.createPart(EditorReference.java:462) at org.eclipse.ui.internal.WorkbenchPartReference.getPart(WorkbenchPartReference.java:595) at org.eclipse.ui.internal.EditorReference.getEditor(EditorReference.java:286) at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditorBatched(WorkbenchPage.java:2857) at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(WorkbenchPage.java:2762) at org.eclipse.ui.internal.WorkbenchPage.access$11(WorkbenchPage.java:2754) at org.eclipse.ui.internal.WorkbenchPage$10.run(WorkbenchPage.java:2705) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2701) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2685) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2668) at org.eclipse.mylyn.internal.tasks.ui.commands.OpenTaskAttachmentInDefaultEditorHandler.openAttachment(OpenTaskAttachmentInDefaultEditorHandler.java:71) at org.eclipse.mylyn.internal.tasks.ui.commands.OpenTaskAttachmentInDefaultEditorHandler.execute(OpenTaskAttachmentInDefaultEditorHandler.java:54) at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:294) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:508) at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:169) at org.eclipse.ui.internal.handlers.SlaveHandlerService.executeCommand(SlaveHandlerService.java:241) at org.eclipse.ui.internal.handlers.SlaveHandlerService.executeCommand(SlaveHandlerService.java:241) at org.eclipse.ui.menus.CommandContributionItem.handleWidgetSelection(CommandContributionItem.java:770) at org.eclipse.ui.menus.CommandContributionItem.access$10(CommandContributionItem.java:756) at org.eclipse.ui.menus.CommandContributionItem$5.handleEvent(CommandContributionItem.java:746) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:3542) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1247) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1270) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1255) at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1076) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3440) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3099) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2405) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2369) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2221) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:500) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:493) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:592) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:559) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:514) at org.eclipse.equinox.launcher.Main.run(Main.java:1311) at org.eclipse.equinox.launcher.Main.main(Main.java:1287) > I also plan to get rid of attachment URLs since they cause problems in my > connector too :-) so I had this goal in mind when working on this bug.
I made a new test issue with some xml files as attachments and I can confirm that these open with the default editor (fragment.xml with the dedicated editor, another xml with a plain text editor. So the thing to improve is opening files for which no default editor is present (like JPG and PNG), should these be handed of to the browser, or should it be delegated to the OS, download, create and then open local copy?
Thank you Maarten for spending more time on this. I am little worried about the stacktrace you sent, because my patch (v3) removed "OpenTaskAttachmentInDefaultEditorHandler" class (as well as declaration of handler in plugin.xml in mylyn.tasks.ui plugin). My patch also moved "Open With Browser" menu item into "Open With" submenu (filled by OpenTaskAttachmentWithMenu class). (In reply to comment #22) > > Gives the follwoing stacktrace on opening the browser window: > org.eclipse.ui.PartInitException: Could not open Web browser on > project-step21.png. Ensure that it is an uncompressed file. ... > at org.eclipse.mylyn.internal.tasks.ui.commands.OpenTaskAttachmentInDefaultEditorHandler.openAttachment(OpenTaskAttachmentInDefaultEditorHandler.java:71) > at > org.eclipse.mylyn.internal.tasks.ui.commands.OpenTaskAttachmentInDefaultEditorHandler.execute(OpenTaskAttachmentInDefaultEditorHandler.java:54) ... This handler should not be used at all :-( (In reply to comment #23) > So the thing to improve is opening files for which no default editor is present > (like JPG and PNG), should these be handed of to the browser, or should it be > delegated to the OS, download, create and then open local copy? In comment 11 we agreed that if there is no default editor, attachment is opened by browser. If attachment doesn't have an URL, other editors are tried too (i.e. external system editor).
*** Bug 293164 has been marked as a duplicate of this bug. ***
Great stuff, Peter. I apologize for the long delay in reviewing your patch. I have now requested an IP review by the Eclipse Foundation (CQ 3789) and we'll hopefully here back soon so we can get this change applied for the 3.4 release.
As requested by private IP Review process: I declare that I wrote all the code in the patch, although I read existing eclipse code related to opening editors by content type and probably used some ideas. I have right to donate this code to the Eclipse, and patch is under the EPL.
Thanks Peter. The CQ got approved. I'm planning to get the patch applied within the next three weeks.
I have applied the patch with minor modifications. Great stuff, Peter! I left the browser as the default editor since browsers generally support a large number of file types and Eclipse does not have a built-in image viewer for instance. I also added some separators to the open with menu to mirror the presentation of the package explorer's open with menu.
This looks great! I converted this (and the ApplyPatchAction) to use the progress service runInUI instead of being a job since it is a bit odd that when the user performs an action that it just runs in the background and then will randomly pop up with the result.
