5202 - Internationalization for breadcrumbs , alert messages

[JayanthyChengan]

Related Issues

• connects to Internationalization - alert messages is not toggling with language selection #5202

Pull Request Checklist

• Merged latest from "develop" branch

[coveralls]

Coverage increased (+0.002%) to 17.683% when pulling de49517 on scholarsportal:5202-alert-messages into 881694b on IQSS:develop.

[mheppler]

When researching this approach (http://jdevelopment.nl/internationalization-jsf-utf8-encoded-properties-files/) it appears that the html tag has the lang attribute using the same dynamic value. Our xhtml pgs do not have this attribute, but instead it is found on the dataverse_template.xhtml file. This PR should also review the use of this attribute and what is best moving forward. Will review with @scolapasta for a more detailed recommendation.

[mheppler]

While reviewing the use of lang in the dataverse_template.xhtml, we should also look at the meta element http-equiv="Content-Language" content="en". According to the W3C this should no longer be used in HTML5 (https://www.w3.org/International/questions/qa-html-language-declarations).

[mheppler]

After a quick test locally, it appears that the lang html attribute is required in the dataverses_template.xhtml file, because it does not get passed to the browser source code if it defined on the individual xhtml pgs like this ThemeAndWidgets.xhtml pg.

[mheppler]

@JayanthyChengan, Thank you for your efforts in delivering this functionality. Sorry for the delay in getting you more concrete feedback, but I have been reviewing your PR changes with @scolapasta and learning more about the f:view tag as it is applied here.

Gustavo and I agree with your approach of applying the f:view tag to each page, as it appears to be the only method we could find to apply the language change to the alert messages delivered by the message.xhtml custom component pg. We had hoped that applying it to the dataverse-template.xhtml would be a more efficient way to solve this functionality being applied to every page in the app, but it didn't appear to work. The f:view that is on that page currently isn't sufficient, and we tried moving it around, to no avail. So we can move forward with this approach on every page as a workaround until we find a cleaner, more scalable solution.

Here is our list of to-do's based on our code review.

• Remove <f:view locale="#{dataverseLocaleBean.localeCode}">...</f:view> from the dataverse-template.xhtml as it won't be needed with it being applied to every pg
• Change lang attribute in the html tag of the dataverse-template.xhtml to lang="#{dataverseLocaleBean.localeCode}"
• Add xml:lang="#{dataverseLocaleBean.localeCode}" attribute to html tag of the dataverse-template.xhtml
• Remove deprecated <meta http-equiv="Content-Language" content="en"/> meta element from the h:head of the dataverse-template.xhtml

Also, with the f:view tag being removed from the dataverse-template.xhtml, it will be needed on all pages. Here is an inventory of all the pages, some of which you have already taken care of in this pull request.

• 403.xhtml
• 404.xhtml
• 500.xhtml
• ThemeAndWidgets.xhtml
• confirmemail.xhtml
• dashboard-users.xhtml
• dashboard.xhtml
• dataset-widgets.xhtml
• dataset.xhtml
• dataverse.xhtml
• dataverseuser.xhtml
• editdatafiles.xhtml
• file.xhtml
• guestbook-responses.xhtml
• guestbook.xhtml
• harvestclients.xhtml
• harvestsets.xhtml
• iframe.xhtml
• loginpage.xhtml
• manage-groups.xhtml
• manage-guestbooks.xhtml
• manage-templates.xhtml
• passwordreset.xhtml
• permissions-manage-files.xhtml
• permissions-manage.xhtml
• privateurl.xhtml
• shib.xhtml
• template.xhtml
• /search/advanced.xhtml
• /oauth2/callback.xhtml
• /oauth2/convert.xhtml
• /oauth2/firstLogin.xhtml

[JayanthyChengan]

Thanks @mheppler and @scolapasta . I will do the changes and submit it.

[JayanthyChengan]

@mheppler I have done the changes as mentioned above. Please review the code.

Also note that I haven't added <f:view in dataverse.xhtml, since it results in below error:

[mheppler]

@JayanthyChengan thank you for the updates. I've got two items for you.

First, we need both the lang and the xml:lang attributes in the html on the dataverse_template.xhtml page.

• Add lang="#{dataverseLocaleBean.localeCode}" attribute in the html tag of the dataverse-template.xhtml page

Second, hopefully we can find a solution to add the f:view to the dataverse.xhtml page. At first I did not see the javax.servlet.ServletException: /dataverse.xhtml @10,52 locale=#{dataverseLocaleBean.localeCode}" null error you reported when I was building the branch locally. But after a deleteing and adding my :Languages config setting back to the db, I finally got the error to show. But it went away after a clean and build and a page refresh or two.

Reviewing this issue with @scolapasta, we didn't hit on any solid feedback to give you. Gustavo suspected it to could with the location of the f:view and how/when dataverseLocaleBean is being initialized.

I did some digging on StackOverflow, and this suggestion by BalusC might shed some light on our problem.

Good luck.

[JayanthyChengan]

@mheppler Thanks for the review and stackOverflow link. I have done the changes, please review the code.

[mheppler]

@JayanthyChengan thank you for your latest revisions. I see that the f:view was added to the dataverse pg, but it does not appear to be applied to the info msg on the edit view of the page.

Also, it appears that an unexpected result of the changes to the DataverseLocaleBean.java bean was the language toggle reloading the page entirely. So that if you were on the edit dataverse or edit dataset pg and use the dropdown toggle, the page is reloaded and the user is no longer viewing the edit form.

If you have any ideas or questions, hopefully @scolapasta can better guide from this point on. We've long since past the limits of my JSF know-how.

[scolapasta]

OK, so I did some more digging and have some info that could help:

I tried first to see why the info message on the dataverse page does not translate. I think I know why. If you look at the info message on the Dataset page, it calls:

JH.addMessage(FacesMessage.SEVERITY_INFO, JH.localize("dataset.message.editMetadata"));

while the DataversePage calls:

FacesContext.getCurrentInstance().addMessage(null, new FacesMessage(FacesMessage.SEVERITY_INFO, BundleUtil.getStringFromBundle("dataverse.edit.msg") , BundleUtil.getStringFromBundle("dataverse.edit.detailmsg")));

Notice how Dataset is using JH.localize. That said, if you look at the JSFHelper class, this localize message is deprecated:

/**
 * @deprecated Localization applies not only to the front end (JSF) but also
 * the API so consider using the newer, more flexible BundleUtil methods
 * instead.
 */
@Deprecated
public String localize( String messageKey ) {

So, we can solve the Dataverse page issue using this method (I think), BUT we really should be finding a different solution verall, using the more flexible BundleUtil methods. I also wonder if we changed the code to use these BundleUtil messages (which is what the none messages code already does), if we could then go back to just having the jsf:view on the template, and not every single page.

@JayanthyChengan would you be able to experiment with this? Please let me know if you need more guidance on what to try and I can help figure it out.

[JayanthyChengan]

@scolapasta

1. I am seeing the "edit dataverse" message in the dataverse page (https://user-images.githubusercontent.com/687227/49038072-de77b200-f189-11e8-9000-2afc0cc5bfbb.png) is coming from dv_rebind_bootstrap_ui.js" , where internationalization is not implemented in javascript.

function post_edit_dv_general_info(){ show_info_msg('Edit Dataverse', 'Edit your dataverse and click Save Changes. Asterisks indicate required fields.'); hide_search_panels(); bind_bsui_components(); }

2. noticed JH.localize is used roughly in 14 java files. as it is deprecated, do you want me to change it to BundleUtil in all places? I guess this would be a separate issue.

3. as @mheppler mentioned

if you were on the edit dataverse or edit dataset pg and use the dropdown toggle, the page is reloaded and the user is no longer viewing the edit form.

Since the URL(http://localhost:8080/dataset.xhtml?persistentId=doi:10.5072/FK2/NS8XGA) remains same in both viewing dataset and editing dataset, so, when the language toggle is changed, it's reloading the page to viewing mode, how to handle this behaviour?

[mheppler]

@JayanthyChengan thank you for catching that msg was originating from the dv_rebind_bootstrap_ui.js and sorry I did not pick that up. Also my apologies for my confusion related to the language toggle page refresh. I was thinking of the behavior on the Manage Harvesting Clients pg, which continues to display the info msg after you toggle because it is it's own independent pg, as opposed to an "edit mode" view that the dataset and dataverse pgs use.

With that said, we can scratch item 3 from your list above, as my mistake. @scolapasta is going to provide some feedback for item 2. That leaves item 1, which I have fixed for you! You should see that I have created a new branch with the changes to the dataverse pg, and sent a pull request to your branch.

I was able to remove the text from the javascript, and changed the dataverse pg to instead receive the msg from the bundle via the DataversePage.java manage bean like the rest of the pages do.

One suggestion that I will make about the French bundle is that now there are two hyphens and no trailing period on the last sentence. I didn't want to edit the bundle myself, but instead thought a note here would be more appropriate.

Let me know if you have any questions. I have also made a comment regarding the changes in the issue for javascript Internationalization #4679. (Also added a reference to a fix for a few of the other text+javascript examples in your comment!)

[JayanthyChengan]

@mheppler Thanks for the detailed explanation and pull request. I will take care of french bundle file.

[scolapasta]

@JayanthyChengan I would make the changes to not use localize in this PR. Since that helper is used only for messages (I'm pretty sure this is true) and that what's this issue is for, it makes sense to me to do this fully the first time. I also think that this may allow for the jsf:view to only be on the template* and would appreciate if you could experiment some with that to get it to work.

• the reason I suspect this is that this only uses:
  Locale locale = facesContext.getViewRoot().getLocale();
  to get the locale whereas I think the session scoped bean will have it captured.

[JayanthyChengan]

@scolapasta
In this PR #5358,

• replaced JH.localize in all java programs with BundleUtil.getStringFromBundle()
• added <f:view only for template ( tried with adding xml:lang , @lang to html tag)

src/main/webapp/dataverse_template.xhtml
src/main/webapp/manage-templates.xhtml
src/main/webapp/template.xhtml

But I don't see the language toggle in the messages and breadcrumbs , for example in dataverseuser.xhtml
(Why have a Dataverse account? To create your own dataverse and customize it, add datasets, or request access to restricted files)

After adding <f:view to dataverseuser.xhtml page, the messages toggle with language selection

[scolapasta]

@JayanthyChengan so unfortunately, not using localize did not solve the issue with having to use f:view on every page. Oh well, at this point, we can leave that for some other day.

Could you make one of these PRs (or even a new one) have all the changes, and then close the other one(s)? That will make it straightforward to do what should be one final review and then QA.

The final PR should have f:view on all the pages you can navigate to. Also, @mheppler noticed that you added the lang and xml:lang attributes to template.xhtml and manage-template.xhtml and they are not needed there (only needed on dataverse_template.xhtml).

[JayanthyChengan]

@scolapasta @mheppler
closed PR #5358 and merged the code to here.
Please review it, let me know if there is any corrections. Thanks

[mheppler]

Looks good, @JayanthyChengan! Thank you! Moving to QA.

[kcondon]

@JayanthyChengan One down, one to go