You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jspwiki.apache.org by GitBox <gi...@apache.org> on 2021/02/28 13:03:45 UTC

[GitHub] [jspwiki] takalat opened a new pull request #44: JSPWIKI-1117

takalat opened a new pull request #44:
URL: https://github.com/apache/jspwiki/pull/44


   Cookies are now being asked until accepted.
   
   Further discussion:
   - External cookies which are not system required should be parsed in other way to effectively use cookie-policy.
   - Cookie lifetime should be configurable from settings and/or user preferences.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] juanpablo-santos commented on pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
juanpablo-santos commented on pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#issuecomment-796696284


   Hi @takalat,
   
   you could use f.ex. [JSPWIKI-778](https://issues.apache.org/jira/projects/JSPWIKI/issues/JSPWIKI-778); it was opened sometime ago, and it is asking for some specific i18n entries, but I think it should be ok (or we could change the description in order to make it ok ;O)). 
   
   Other issues that may interest to tackle on the scope of your univ. project could be [JSPWIKI-858](https://issues.apache.org/jira/browse/JSPWIKI-858) (the second option noted there) or [JSPWIKI-824](https://issues.apache.org/jira/browse/JSPWIKI-824), but for the current cargo plugin. Those couple of issues are self-contained and don't need an extensive knowledge of JSPWiki internals..
   
   And of course, feel free to reach us for any question at either the [dev ML](https://jspwiki-wiki.apache.org/Wiki.jsp?page=Mailing%20Lists), jira or directly from a PR.
   
   
   best regards,
   juan pablo


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] juanpablo-santos commented on a change in pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
juanpablo-santos commented on a change in pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#discussion_r585068939



##########
File path: jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java
##########
@@ -125,19 +133,21 @@ public static void reloadPreferences( final PageContext pageContext ) {
      * @param request
      * @param prefs The default hashmap of preferences
      */
-	private static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) {
-        final String prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, "JSPWikiUserPrefs" ) );
-        if( prefVal != null ) {
+	private static void parseJSONPreferences( final Preferences prefs, final String prefVal ) {       

Review comment:
       it would be nice if this method is made package-private, so some unit tests defining its behaviour can be made. The alternative would be unit testing the `reloadPreferences` method, but the first approach seems easier.

##########
File path: jspwiki-war/src/main/webapp/templates/default/commonheader.jsp
##########
@@ -156,4 +177,76 @@ String.I18N.PREFIX = "javascript.";
          src="<wiki:Link format='url' templatefile='skins/' /><c:out value='${prefs.SkinName}/skin.js' />" ></script>
 </c:if>
 
+
+
+<%-- Support for cookie acceptance --%>
+ <c:if test='${!(prefs.cookies)}'>
+<script type="text/javascript">
+document.addEventListener("DOMContentLoaded", readyForCookieTest);
+var appBaseName = document.querySelector('meta[name="wikiApplicationName"]').content;
+
+function sendAjaxCookieConfirmation(dialogElem){
+		var request = new XMLHttpRequest();
+		var appBaseUrl = "/"+appBaseName+"/";
+		var cookieAcceptParam = "?cookies=true";
+		request.open('GET', appBaseUrl+cookieAcceptParam, true);
+
+		request.onload = function() {
+		  if (this.status >= 200 && this.status < 400) {
+			// Success!
+			var resp = this.response;
+			console.log("Cookies accepted!");
+			dialogElem.toggle();
+			dialogElem.hide();
+			dialogElem.destroy();
+		  } else {
+			// We reached our target server, but it returned an error
+		  }
+	};
+
+	request.onerror = function() {
+	  // There was a connection error of some sort
+	};	
+		request.send();
+	}
+var cookiesAllow = ${prefs.cookies};
+
+function setCookie(cname, cvalue, exdays) {
+	  var d = new Date();
+	  // Cookie setting lasts for 1 day before deleted from browser.
+	  // TODO: Setup in server preference for client's preferred cookie lifetime.
+	  d.setTime(d.getTime() + (exdays*24*60*60*1000));
+	  var expires = "expires="+ d.toUTCString();
+		document.cookie ="CookiePrefs={"+ cname + ":'" + cvalue + "'}";
+	}
+
+function readyForCookieTest(){
+if(!cookiesAllow){	
+		var cookieDialogHtml = "<div class='dialog-message'>This application uses cookies to work properly. By using the application you understand and accept the use of necessary cookies. Click OK to proceed.</div>";

Review comment:
       Both texts should be i18nized, see [line 90 before](https://github.com/apache/jspwiki/pull/44/files#diff-813bef24eb53b1d4b32778c239d73a4f760fcedccaf84799870302f7c3abef3fR90). Other than that (and the tests before), the PR looks good to me. @brushed WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] takalat commented on a change in pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
takalat commented on a change in pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#discussion_r585901733



##########
File path: jspwiki-war/src/main/webapp/templates/default/commonheader.jsp
##########
@@ -156,4 +177,76 @@ String.I18N.PREFIX = "javascript.";
          src="<wiki:Link format='url' templatefile='skins/' /><c:out value='${prefs.SkinName}/skin.js' />" ></script>
 </c:if>
 
+
+
+<%-- Support for cookie acceptance --%>
+ <c:if test='${!(prefs.cookies)}'>
+<script type="text/javascript">
+document.addEventListener("DOMContentLoaded", readyForCookieTest);
+var appBaseName = document.querySelector('meta[name="wikiApplicationName"]').content;
+
+function sendAjaxCookieConfirmation(dialogElem){
+		var request = new XMLHttpRequest();
+		var appBaseUrl = "/"+appBaseName+"/";
+		var cookieAcceptParam = "?cookies=true";
+		request.open('GET', appBaseUrl+cookieAcceptParam, true);
+
+		request.onload = function() {
+		  if (this.status >= 200 && this.status < 400) {
+			// Success!
+			var resp = this.response;
+			console.log("Cookies accepted!");
+			dialogElem.toggle();
+			dialogElem.hide();
+			dialogElem.destroy();
+		  } else {
+			// We reached our target server, but it returned an error
+		  }
+	};
+
+	request.onerror = function() {
+	  // There was a connection error of some sort
+	};	
+		request.send();
+	}
+var cookiesAllow = ${prefs.cookies};
+
+function setCookie(cname, cvalue, exdays) {
+	  var d = new Date();
+	  // Cookie setting lasts for 1 day before deleted from browser.
+	  // TODO: Setup in server preference for client's preferred cookie lifetime.
+	  d.setTime(d.getTime() + (exdays*24*60*60*1000));
+	  var expires = "expires="+ d.toUTCString();
+		document.cookie ="CookiePrefs={"+ cname + ":'" + cvalue + "'}";
+	}
+
+function readyForCookieTest(){
+if(!cookiesAllow){	
+		var cookieDialogHtml = "<div class='dialog-message'>This application uses cookies to work properly. By using the application you understand and accept the use of necessary cookies. Click OK to proceed.</div>";

Review comment:
       Hi @juanpablo-santos 
   English and Finnish i18ns for cookie html dialog are done in the latest commit.
   
   I'm happy to help!

##########
File path: jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java
##########
@@ -125,19 +133,21 @@ public static void reloadPreferences( final PageContext pageContext ) {
      * @param request
      * @param prefs The default hashmap of preferences
      */
-	private static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) {
-        final String prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, "JSPWikiUserPrefs" ) );
-        if( prefVal != null ) {
+	private static void parseJSONPreferences( final Preferences prefs, final String prefVal ) {       

Review comment:
       I assume this meant only changing signature to package scope. Changed method modifier to <none> in the latest commit.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] brushed commented on pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
brushed commented on pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#issuecomment-791912513


   Looking at the proposed patch,  I'm not in favor of taking this into the
   code base.
   
   
   How does the current JSPWiki Cookie notification work :
   
   In the LeftMenu page,  you will find something like:
   {{{
   [{InsertPage page='Cookie Policy' show='once' section='1' }]
   }}}
   
   Actually,  the "Cookie Policy" page could point to any page
   the user or site administrator wants to use for putting the cookie policy
   description of its wiki.
   
   What the InsertPage plugin does is, it will display the first section of
   that page.
   but only during the first visit of that page. After that, it will remember
   that
   this content has already been shown (via a cookie) and not display it again.
   
   By default the 'show-once' content is presented in a modal dialog box; but
   the
   user can specify other styles. (eg in page information box)
   For more details on the InsertPagePlugin see its documentation page.
   
   
   This means that JSPWiki is not implementing any specific code to show this
   dialog box.  All is driven by the content of your wiki.
   
   EG: The  admin of the site can fully determine what happens
   when a user is presented with the Cookie Policy pop-up.
   What happens when selecting the CANCEL button,  etc.
   
   EG,  if you would be using this wiki on an intranet,  you may not need a
   strong
   cookie policy.  You would simply not include it in your LeftMenu page,  and
   never get any pop-up.
   
   EG, if the admin would be including a Google AD tracker, all he needs to do
   is update its Cookie Policy page.  No changes or rebuild of JSPWiki is
   needed,
   no changes are needed on the I18N properties of JSPWiki etc ..
   
   
   
   *  *  *
   
   JSPWiki always uses Cookies, a user cannot turn it 100% off.
   2 Cookies are always present: one to keep track of your SESSION (needed for
   editing),
   and one to store the user's preferences.
   The initial user preference cookie values are determined by the admin
   setting of your site,
   and are changed whenever the user modifies them.
   
   This means that a user can never "opt-out".
   A user has to accept ("ok understood" button) or click the ('learn more")
   button.
   
   Other cookies managed by JSPWiki (eg created by other InsertPage plugins
   with a show='once')
   can be viewed or deleted on the User Preferences page.
   
   
   
   Grtz,
       dirk
   
   
   
   
   
   
   
   
   
   
   
   On Tue, Mar 2, 2021 at 9:54 PM takalat <no...@github.com> wrote:
   
   > *@takalat* commented on this pull request.
   > ------------------------------
   >
   > In jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java
   > <https://github.com/apache/jspwiki/pull/44#discussion_r585901825>:
   >
   > > @@ -125,19 +133,21 @@ public static void reloadPreferences( final PageContext pageContext ) {
   >       * @param request
   >       * @param prefs The default hashmap of preferences
   >       */
   > -	private static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) {
   > -        final String prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, "JSPWikiUserPrefs" ) );
   > -        if( prefVal != null ) {
   > +	private static void parseJSONPreferences( final Preferences prefs, final String prefVal ) {
   >
   > I assume this meant only changing signature to package scope. Changed
   > method modifier to in the latest commit.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/jspwiki/pull/44#discussion_r585901825>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AA5QPBYBKL6OXTR6TQ44DVTTBVGA5ANCNFSM4YLBIHKA>
   > .
   >
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] juanpablo-santos closed pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
juanpablo-santos closed pull request #44:
URL: https://github.com/apache/jspwiki/pull/44


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] takalat commented on pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
takalat commented on pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#issuecomment-796656255


   > @takalat since you seem to know finnish :-) one quick contribution that could be made, and would be greatly appreciated, is to complete the associated [i18n fi files](https://jspwiki-wiki.apache.org/Wiki.jsp?page=HowToI18n). According to the [translation status page](https://jspwiki.apache.org/development/i18n.html) they're a bit behind..
   > 
   > thx + best regards,
   > juan pablo
   
   @juanpablo-santos 
   a Q about the fi_i8ns:
   
   Would it be possible to have an issue about fi_i8ns (improvement) task ?
   
   We are having currently a uni. project on open-source project. One requirement from behalf of the assignment is that we have an issue listed on a issue tracker for having an approval of the work done.
   
   I guess we could then check that all fi-localizations are up-to-date. We have one finnish language student also in the group, who's familiar on digital tools language translations.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] takalat commented on a change in pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
takalat commented on a change in pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#discussion_r585901825



##########
File path: jspwiki-main/src/main/java/org/apache/wiki/preferences/Preferences.java
##########
@@ -125,19 +133,21 @@ public static void reloadPreferences( final PageContext pageContext ) {
      * @param request
      * @param prefs The default hashmap of preferences
      */
-	private static void parseJSONPreferences( final HttpServletRequest request, final Preferences prefs ) {
-        final String prefVal = TextUtil.urlDecodeUTF8( HttpUtil.retrieveCookieValue( request, "JSPWikiUserPrefs" ) );
-        if( prefVal != null ) {
+	private static void parseJSONPreferences( final Preferences prefs, final String prefVal ) {       

Review comment:
       I assume this meant only changing signature to package scope. Changed method modifier to default modifier in the latest commit.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] juanpablo-santos commented on pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
juanpablo-santos commented on pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#issuecomment-792357329


   closed JSPWIKI-1117 with won't fix; after looking closely at JSPWIKI-1147. it is unrelated to the cookie noted before and it's a different bug on its own..


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [jspwiki] juanpablo-santos commented on pull request #44: JSPWIKI-1117

Posted by GitBox <gi...@apache.org>.
juanpablo-santos commented on pull request #44:
URL: https://github.com/apache/jspwiki/pull/44#issuecomment-792273694


   Hi @brushed ,
   
   thanks for the detailed response! Given that I think we can close both JSPWIKI-1117 and JSPWIKI-1147 with "Won't fix". 
   
   @takalat since you seem to know finnish :-) one quick contribution that could be made, and would be greatly appreciated, is to complete the associated [i18n fi files](https://jspwiki-wiki.apache.org/Wiki.jsp?page=HowToI18n). According to the [translation status page](https://jspwiki.apache.org/development/i18n.html) they're a bit behind..
   
   thx + best regards,
   juan pablo


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org