Alexander Wels has posted comments on this change. Change subject: webadmin: Added handling missing languages ......................................................................
Patch Set 1: Verified (10 inline comments) .................................................... File backend/manager/modules/root/pom.xml Line 65: <groupId>javax.servlet</groupId> Line 66: <artifactId>jstl</artifactId> Line 67: <version>1.2</version> Line 68: <scope>provided</scope> Line 69: </dependency> Done Line 70: <dependency> Line 71: <groupId>org.mockito</groupId> Line 72: <artifactId>mockito-all</artifactId> Line 73: <version>1.9.5</version> Line 71: <groupId>org.mockito</groupId> Line 72: <artifactId>mockito-all</artifactId> Line 73: <version>1.9.5</version> Line 74: <scope>test</scope> Line 75: </dependency> Done Line 76: </dependencies> Line 77: Line 78: <build> Line 79: Line 71: <groupId>org.mockito</groupId> Line 72: <artifactId>mockito-all</artifactId> Line 73: <version>1.9.5</version> Line 74: <scope>test</scope> Line 75: </dependency> Done Line 76: </dependencies> Line 77: Line 78: <build> Line 79: .................................................... File backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsServlet.java Line 45: file = checkForIndex(request, response, file, request.getPathInfo()); Line 46: if(null == file) { Line 47: response.sendError(HttpServletResponse.SC_NOT_FOUND); Line 48: } else if(!response.isCommitted()){ //If the response is committed, we have already redirected. Line 49: Object languagePageShown = request.getSession(true).getAttribute(LANG_PAGE_SHOWN); The problem the attribute might not have been set, and thus the result is null. You can't cast a null to a boolean. Line 50: if(!file.equals(originalFile)) { Line 51: //We determined that we are going to redirect the user to the english version URI. Line 52: String redirect = request.getServletPath() + replaceLocaleWithUSLocale(request.getPathInfo(), locale); Line 53: if((null == languagePageShown || !Boolean.parseBoolean(languagePageShown.toString()))) { Line 50: if(!file.equals(originalFile)) { Line 51: //We determined that we are going to redirect the user to the english version URI. Line 52: String redirect = request.getServletPath() + replaceLocaleWithUSLocale(request.getPathInfo(), locale); Line 53: if((null == languagePageShown || !Boolean.parseBoolean(languagePageShown.toString()))) { Line 54: request.getSession(true).setAttribute(LANG_PAGE_SHOWN, true); See my other comment, AFTER I have done this, the cast is fine, however the cast happens before the first time this is called. Line 55: request.setAttribute(LOCALE, locale); Line 56: request.setAttribute(ENGLISH_HREF, redirect); Line 57: RequestDispatcher dispatcher = getServletContext().getRequestDispatcher("/help/no_lang.jsp"); Line 58: if (dispatcher != null) { Line 53: if((null == languagePageShown || !Boolean.parseBoolean(languagePageShown.toString()))) { Line 54: request.getSession(true).setAttribute(LANG_PAGE_SHOWN, true); Line 55: request.setAttribute(LOCALE, locale); Line 56: request.setAttribute(ENGLISH_HREF, redirect); Line 57: RequestDispatcher dispatcher = getServletContext().getRequestDispatcher("/help/no_lang.jsp"); Of course, D'oh! I can't believe I missed that. Will fix that soon. Line 58: if (dispatcher != null) { Line 59: dispatcher.include(request, response); Line 60: } Line 61: } else { Line 87: Line 88: private String replaceLocaleWithUSLocale(String originalString, Locale locale) { Line 89: //Have to check against both the _ and - versions of the locale to replace, as they might not be consistent. Line 90: return originalString.replaceAll("/"+ locale.toLanguageTag().replaceAll("-", "\\\\-") + "|/" + locale.toString(), Line 91: "/" + Locale.US.toLanguageTag()); Sure makes no difference to the compiler, I am sure it will optimize the extra variable away. Line 92: } Line 93: Line 94: /** Line 95: * Determines the locale based on the request passed in. It will first try to determine the locale .................................................... File backend/manager/modules/root/src/main/webapp/help/no_lang.jsp Line 20: <div class="titlepage"> Line 21: <div> Line 22: <p>It appears that you do not have the <c:out value="${requestScope['locale'].getDisplayLanguage()}" /> language pack installed. Line 23: Please have your administrator install the proper language by running:</p> Line 24: <pre class="screen"><code class="command">yum install rhevm-doc-<c:out value="${requestScope['locale'].toLanguageTag()}" /></code></pre> Good point, will keep it generic here, and if needed add a more specific message for RHEV-M Line 25: <p> Line 26: Please click <a href="${requestScope['englishHref']}">here</a> for the English documentation. This message will Line 27: only be displayed once per session. Line 28: </p> .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ServletUtils.java Line 153: // All checks passed, the path is sane: Line 154: return true; Line 155: } Line 156: Line 157: public static File getFileFromString(String path, File base) { Done Line 158: File file = null; Line 159: Line 160: if (path == null) { Line 161: file = base; Line 160: if (path == null) { Line 161: file = base; Line 162: } Line 163: else if (!ServletUtils.isSane(path)) { Line 164: log.error("The path \"" + path + "\" is not sane, will send a 404 error response."); Done Line 165: } Line 166: else { Line 167: file = new File(base, path); Line 168: } -- To view, visit http://gerrit.ovirt.org/10065 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37156061cbdd7d2df3290c88dee933c41e0087c5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches