Alon Bar-Lev has posted comments on this change. Change subject: engine: no documentation message ......................................................................
Patch Set 6: (7 comments) Starting to understand!!! .................................................... File backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsMissingServlet.java I may be wrong... long time since I developed anything for the web. but if you just put jsp in the war it will just work... no need for this redirection servlet. and if so important to have this APPLICATION_TYPE attribute, you can pass init parameter to jsp as any servlet. and... let's say this is required, the NO_LOCALE_JSP can be init parameter so we can reuse. but I really don't think we need this servlet. Line 1: package org.ovirt.engine.core; Line 2: Line 3: import java.io.IOException; Line 4: .................................................... File backend/manager/modules/root/src/main/webapp/WEB-INF/help/no_lang.jsp Line 1: <%@ page pageEncoding="UTF-8" session="false" %> Line 2: <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> Line 3: <%@ taglib prefix="fmt" uri="http://java.sun.com/jsp/jstl/fmt" %> Line 4: <fmt:setBundle basename="org.ovirt.engine.core.docswarning" var="docs"/> I suggest bundle to be "root" per the root war, so that all messages of root be in the same bundle. Line 5: <!DOCTYPE html> Line 6: <html> Line 7: <head> Line 8: <meta http-equiv="Content-type" content="text/html; charset=utf-8" /> Line 5: <!DOCTYPE html> Line 6: <html> Line 7: <head> Line 8: <meta http-equiv="Content-type" content="text/html; charset=utf-8" /> Line 9: <link rel="shortcut icon" href="${pageContext.request.contextPath}/ovirt-engine-theme-resource/favicon" type="image/x-icon" /> the icon paste can be in some utility so we can reuse. Line 10: <title><fmt:message key="docs.missing.title" bundle="${docs}"> Line 11: <fmt:param value="${requestScope['locale'].getDisplayLanguage()}" /> Line 12: </fmt:message> Line 13: </title> Line 13: </title> Line 14: <c:if test="${requestScope['brandingStyle'] != null}"> Line 15: <c:forEach items="${requestScope['brandingStyle']}" var="theme"> Line 16: <c:if test="${theme.getThemeStyleSheet(requestScope['applicationType']) != null}"> Line 17: <link rel="stylesheet" type="text/css" href="${pageContext.request.contextPath}/ovirt-engine-theme${theme.path}/${theme.getThemeStyleSheet(requestScope['applicationType'])}"> where the theme is (/ovirt-engine-theme) should be init parameter or better application context attribute. In other words, it should be only specified at web.xml. Line 18: </c:if> Line 19: </c:forEach> Line 20: </c:if> Line 21: </head> Line 16: <c:if test="${theme.getThemeStyleSheet(requestScope['applicationType']) != null}"> Line 17: <link rel="stylesheet" type="text/css" href="${pageContext.request.contextPath}/ovirt-engine-theme${theme.path}/${theme.getThemeStyleSheet(requestScope['applicationType'])}"> Line 18: </c:if> Line 19: </c:forEach> Line 20: </c:if> hmmm... can't the above be in some common utility? Line 21: </head> Line 22: <body> Line 23: <div> Line 24: <div class="obrand_left"> .................................................... File backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml Line 65: <param-name>file</param-name> Line 66: <param-value>%{ENGINE_MANUAL}</param-value> Line 67: </init-param> Line 68: <init-param> Line 69: <param-name>forwardPath</param-name> forward? maybe missing? Line 70: <param-value>/ovirt-engine/docsmissing</param-value> Line 71: </init-param> Line 72: </servlet> Line 73: <servlet-mapping> Line 200: Line 201: <filter-mapping> Line 202: <filter-name>BrandingFilter</filter-name> Line 203: <url-pattern>/ovirt-engine</url-pattern> Line 204: <url-pattern>/ovirt-engine/docsmissing</url-pattern> instead of each location, can we have a URI namespace? I know this is part of bug#, but no reason to force people to write explicit servlets. Line 205: <dispatcher>FORWARD</dispatcher> Line 206: <dispatcher>REQUEST</dispatcher> Line 207: </filter-mapping> Line 208: -- To view, visit http://gerrit.ovirt.org/19848 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iceae0ebf671efc951522c464db1a5b2b95dd5637 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches