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 <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
