[ 
https://issues.apache.org/jira/browse/FELIX-1015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12719948#action_12719948
 ] 

Felix Meschberger commented on FELIX-1015:
------------------------------------------

Thanks for providing the patch which looks good.

I just have a few questions:

* How about making the brandingPlugin field static in AbstractWebConsole ? This 
way it just needs to be set once instead of on all instances.

* As a consequence the plugins field in OSGiManager does not need to be static 
(does not need to be anyway, since it can be accessed through the instance, 
which is available in the tracker). Also the OSGiManager does not need to have 
a brandingPlugin field. Since it suffices it to set it on the 
AbstractWebConsolePlugin class.

* The AbstractWebConsolePlugin.setBrandingPlugin method might be extended to 
reset to the DefaultBrandingPlugin if the brandingplugin being set is null. 
This way the decision on using the DefaultBrandingPlugin remains in the 
AbstractWebConsolePlugin and the 
OsgiManager.BrandingServiceTracker.removedService method just "sends" null to 
the AbstractWebConsolePlugin.


> Hardcoded HTML Header/Footer in AbstractWebConsolePlugin
> --------------------------------------------------------
>
>                 Key: FELIX-1015
>                 URL: https://issues.apache.org/jira/browse/FELIX-1015
>             Project: Felix
>          Issue Type: Sub-task
>          Components: Web Console
>    Affects Versions: webconsole-1.2.8
>            Reporter: Thomas Diesler
>            Assignee: Felix Meschberger
>         Attachments: branding.patch
>
>
> Instead of 
>     private static final String HEADER = "<?xml version=\"1.0\" 
> encoding=\"UTF-8\" ?>"
>         + "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\" 
> \"xhtml1-transitional.dtd\">"
>         + "<html xmlns=\"http://www.w3.org/1999/xhtml\";>"
>         + "<head>"
>         + "<meta http-equiv=\"Content-Type\" content=\"text/html; utf-8\">"
>         + "<link rel=\"icon\" href=\"{6}/res/imgs/favicon.ico\">"
>         + "<title>{0} - {2}</title>"
>         + "<script src=\"{5}/res/ui/jquery-1.3.2.min.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/jquery.tablesorter-2.0.3.min.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/admin.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/ui.js\" language=\"JavaScript\"></script>"
>         + "<script language=\"JavaScript\">"
>         + "appRoot = \"{5}\";"
>         + "pluginRoot = appRoot + \"/{6}\";"
>         + "</script>"
>         + "<link href=\"{5}/res/ui/admin.css\" rel=\"stylesheet\" 
> type=\"text/css\">"
>         + "</head>"
>         + "<body>"
>         + "<div id=\"main\">"
>         + "<div id=\"lead\">"
>         + "<h1>"
>         + "{0}<br>{2}"
>         + "</h1>"
>         + "<p>"
>         + "<a target=\"_blank\" href=\"{3}\" title=\"{1}\"><img 
> src=\"{5}/res/imgs/logo.png\" width=\"165\" height=\"63\" border=\"0\"></a>"
>         + "</p>" + "</div>";
> we propose 
>     protected String getHeader()
>     {
>        String HEADER = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>"
>         + "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\" 
> \"xhtml1-transitional.dtd\">"
>         + "<html xmlns=\"http://www.w3.org/1999/xhtml\";>"
>         + "<head>"
>         + "<meta http-equiv=\"Content-Type\" content=\"text/html; utf-8\">"
>         + "<link rel=\"icon\" href=\"{6}/res/imgs/favicon.ico\">"
>         + "<title>{0} - {2}</title>"
>         + "<script src=\"{5}/res/ui/jquery-1.3.2.min.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/jquery.tablesorter-2.0.3.min.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/admin.js\" 
> language=\"JavaScript\"></script>"
>         + "<script src=\"{5}/res/ui/ui.js\" language=\"JavaScript\"></script>"
>         + "<script language=\"JavaScript\">"
>         + "appRoot = \"{5}\";"
>         + "pluginRoot = appRoot + \"/{6}\";"
>         + "</script>"
>         + "<link href=\"{5}/res/ui/admin.css\" rel=\"stylesheet\" 
> type=\"text/css\">"
>         + "</head>"
>         + "<body>"
>         + "<div id=\"main\">"
>         + "<div id=\"lead\">"
>         + "<h1>"
>         + "{0}<br>{2}"
>         + "</h1>"
>         + "<p>"
>         + "<a target=\"_blank\" href=\"{3}\" title=\"{1}\"><img 
> src=\"{5}/res/imgs/logo.png\" width=\"165\" height=\"63\" border=\"0\"></a>"
>         + "</p>" + "</div>";
>        return HEADER;
>     }
> -------------
>     protected PrintWriter startResponse( HttpServletRequest request, 
> HttpServletResponse response ) throws IOException
>     {
>         ...
>         String header = MessageFormat.format( getHeader(), new Object[]
>     }
>     protected void endResponse( HttpServletRequest request, PrintWriter pw )
>     {
>         pw.println( "</body>" );
>         pw.println( "</html>" );
>     }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to