Comment 1 Steffen Pingel 2009-12-09 21:51:44 EST

Thanks for the request Steve. I think that would make a lot of sense. I would go a bit further and delegate opening of task, query, search URLs etc. in a browser to the connector entirely. Some repositories may require posting to a form or a cookie to authenticate for instance which can not be encoded in a URL. 

Here is a fist draft of a class that would be registered per connectorKind through an extension point:

 abstract class AbstractOpenRepositoryElementHandler {

  Status openInBrowser(TaskRepository repository, IRepositoryElement element);
 
  URL getAuthenticatedUrl(TaskRepository repository, IRepositoryElement element);

 }

The default implementation of openInBrowser() could simply open the authenticated URL but clients could override to do something more fancy. Would that satisfy your requirements?

Comment 2 Steve Elsemore 2009-12-10 09:43:33 EST

Thanks, Steffen.  This would work for us, provided that the getTaskUrl connector method could still be used to get an unauthenticated URL (to copy to clipboard, for example).  But I'm curious why you would go with an extension point rather than introducing the two new methods to AbstractRepositoryConnectorUi (with default implementations that would maintain the current behavior)?

Comment 3 Steffen Pingel 2010-01-29 18:02:13 EST

The advantage of an extension point would be that there could be implementations provided by 3rd parties that would not be coupled to the repository connector. I have had a quick with Shawn about this and it's probably best if we go with a more simple approach for now since there are quite a few cases to consider for proper browser support (e.g. when using external browsers) so we may initially only support authentication through URLs and worry about form based authentication later:

public abstract class AbstractRepositoryConnector {

  public URL getAuthenticatedUrl(TaskRepository repository, IRepositoryElement
element) {
    return null;
  }

}

Steve, would you be interested in providing a patch that adds this method and modifies the code that opens tasks accordingly?

Comment 4 Steve Elsemore 2010-01-29 18:36:48 EST

Steffen,

Sure, I'll be glad to take a look at it.

Steve

Comment 5 Steve Elsemore 2010-02-02 12:38:34 EST

Created attachment 157931 [details]
getAuthenticatedUrl implementation

This patch (core, ui) adds the getAuthenticatedUrl method to AbstractRepositoryConnector, and modifies the code that opens tasks.  If the connector has overridden getAuthenticatedUrl to return a non-null URL, then that URL will be used.  Otherwise, the task URL will be used (same as current behavior).

Comment 6 Steffen Pingel 2010-02-28 22:29:37 EST

Thanks Steve! I have applied your patch with minor modifications: I extracted some of the common code to TasksUiInternal and the method for opening repository elements is now called TasksUiUtil.openWithBrowser(). 

Please take a look the changes and let me know if the implementation works for you.

Comment 7 Steve Elsemore 2010-03-01 10:20:58 EST

Steffen,

Thanks, that will be perfect.

Steve

Comment 8 Steffen Pingel 2010-03-01 13:04:38 EST

Thanks for the feedback and your contribution. I'll mark this as closed. Please reopen if you run into any problems.