Allow cookies be set for textarea heights upon resize and restore upon document load

[Carsten Klein <carsten.klein@…>]

This basically involves incorporating the jquery plugin for handling cookies.

The patch is rather simple. See the attached jquery_cookies.js which must be included with trac/htdocs/js/.

Here is the original project page for reference:

​http://plugins.jquery.com/project/cookie

htdocs/js/resizer.js

@@ -5 +5 @@
     var textarea = $(this);
     var offset = null;
     
+    function getCookieName(textarea) {
+      if (textarea.attr("id")) {
+        return "trac." + textarea.attr("id");
+      }
+      else if (textarea.attr("name")) {
+        return "trac." + textarea.attr("name");
+      }
+    }
+    
     function beginDrag(e) {
       offset = textarea.height() - e.pageY;
       textarea.blur();
@@ -18 +27 @@
     }
     
     function endDrag(e) {
+      // set cookie for permanently storing information 
+      // on the height of the resizable, if possible
+      name = getCookieName(textarea);
+      if (name) {
+        $.cookie(name, textarea.height(), ( new Date() ).getTime() + 1000000);          
+      }
       textarea.focus();
       $(document).unbind('mousemove', dragging).unbind('mouseup', endDrag);
     }
@@ -27 +42 @@
             .parent().append(grip);
     grip.style.marginLeft = (this.offsetLeft - grip.offsetLeft) + 'px';
     grip.style.marginRight = (grip.offsetWidth - this.offsetWidth) +'px';
+    
+    // restore height from cookie, if avail
+    var height = 0;
+    var name = getCookieName(textarea);
+    if (name)
+    {
+      height = $.cookie(name)
+    }
+    textarea.height(Math.max(32, height));
   });
 });
+

web/chrome.py

@@ -522 +522 @@
         add_link(req, 'help', req.href.wiki('TracGuide'))
         add_stylesheet(req, 'common/css/trac.css')
         add_script(req, 'common/js/jquery.js')
+        add_script(req, 'common/js/jquery_cookies.js')
         # Only activate noConflict mode if requested to by the handler
         if handler is not None and \
            getattr(handler.__class__, 'jquery_noconflict', False):

BTW Thanks for incorporating this, it makes using trac a lot more easier.

[Carsten Klein <carsten.klein@…>]

The jquery cookies plugin

[Carsten Klein <carsten.klein@…>]

Issues might be raised on non secure cookies set by the above provided patch. So additional tests must be run in order to make it work under all conditions, e.g. ie8+ requires secure cookie domains and so on.

[Carsten Klein <carsten.klein@…>]

*Sigh*, the patch is missing a semicolon in line 51 on height = $.cookie(name). However, since it is the sole statement in that block it eventually works just fine. Feel free to add missing punctuation at your own expense ;)

[Remy Blank]

Good idea! It would be great if you could complete and test the implementation according to your concerns in comment:1.

Also, you don't really need getCookieName(). Just calculate the name once at the top level and store the result in a local, the same as textarea. And the expires argument to $.cookie() can be a number of days, so you don't have to make the calculation yourself. Finally, I would add the jquery_cookies.js script in Chrome.add_textarea_grips() instead of including it unconditionally.

[Carsten Klein <carsten.klein@…>]

Nice of you. I will look further into this. As of now, all cookies established by the above patch are still session bound, so that the setting will be lost on user agent exit.

I will revise the patch according to your input.

Apart from that, I would rather see the cookies plugin be available on a general basis, and not just on add_text_area_grips(), because that would add extra functionality to 3rd party plugins, don't you think?

Besides, providing a compressed version of the plugin would reduce overall bandwidth, also.

[Carsten Klein <carsten.klein@…>]

Reconsidering this, perhaps make this an additional call to the chrome api, e.g. add_cookies_support() or something like that.

[Carsten Klein <carsten.klein@…>]

regarding comment:1 - i do not have nor run windows except for gaming purposes ;)

[Carsten Klein <carsten.klein@…>]

Replying to Carsten Klein <carsten.klein@…>:

Reconsidering this, perhaps make this an additional call to the chrome api, e.g. add_cookies_support() or something like that.

Macros would definitely require this, i presume. So, the cookies should be available by default, since AFAIK the macros cannot instruct chrome to add support for cookies, or am I wrong?

[Remy Blank]

Replying to Carsten Klein <carsten.klein@…>:

Apart from that, I would rather see the cookies plugin be available on a general basis, and not just on add_text_area_grips(), because that would add extra functionality to 3rd party plugins, don't you think?

Plugins are free to add_script(req, 'common/js/jquery_cookies.js') if they need it. jQuery (core) is only included unconditionally because virtually all pages need it.

[Carsten Klein <carsten.klein@…>]

How many days would be appropriate for the cookie to remain valid?

Or should the cookie be refreshed on every load of the page so that it will remain valid for at least 365 days, and if the user once again opens the page, it will be refreshed so that it will remain valid for the next 365 days?

[Carsten Klein <carsten.klein@…>]

Here is a corrected version of the patch. It will automatically refresh existing cookies so that on page load the lifetime of the cookie will be extended even if the user does not alter the height property of the textareas on that page.

In addition, only those request handlers that require the jquery_cookies.js plugin to be present will include it. Basically this could be done on the template level, also. However, since genshi processing overhead is still subject to discussion, it is included directly in the respective request handler's handle_request() methods.

The naming of the individual cookies is subject to discussion, since the data must be transferred to the server and back shorter names would actually make sense here.

trac/ticket/web_ui.py

@@ -163 +163 @@
             return True
 
     def process_request(self, req):
+
+        add_script(req, 'common/js/jquery_cookies.js')
+
         if 'id' in req.args:
             if req.path_info == '/newticket':
                 raise TracError(_("id can't be set for a new ticket request."))

trac/ticket/roadmap.py

@@ -581 +582 @@
             milestone.name = milestone_id
             action = 'edit' # rather than 'new' so that it works for POST/save
 
+        add_script(req, 'common/js/jquery_cookies.js')
+
         if req.method == 'POST':
             if req.args.has_key('cancel'):
                 if milestone.exists:

trac/htdocs/js/resizer.js

@@ -4 +4 @@
   $('textarea.trac-resizable').each(function() {
     var textarea = $(this);
     var offset = null;
-    
+    var stateCookieName = null;
+
+    if (textarea.attr("id")) {
+      stateCookieName = "trac_ui.textarea." + textarea.attr("id") + ".height";
+    }
+    else if (textarea.attr("name")) {
+      stateCookieName = "trac_ui.textarea." + textarea.attr("name") + ".height";
+    }
+
     function beginDrag(e) {
       offset = textarea.height() - e.pageY;
       textarea.blur();
@@ -18 +26 @@
     }
     
     function endDrag(e) {
+      // set cookie for permanently storing information 
+      // on the height of the resizable, if possible
+      if (stateCookieName) {
+        $.cookie(stateCookieName, textarea.height(), { expires: 365 });         
+      }
       textarea.focus();
       $(document).unbind('mousemove', dragging).unbind('mouseup', endDrag);
     }
@@ -27 +40 @@
             .parent().append(grip);
     grip.style.marginLeft = (this.offsetLeft - grip.offsetLeft) + 'px';
     grip.style.marginRight = (grip.offsetWidth - this.offsetWidth) +'px';
+    
+    // restore height from cookie, if avail
+    // and refresh cookie so that it stays valid
+    var height = Math.max(32, textarea.height());
+    if (stateCookieName) {
+      height = $.cookie(stateCookieName);
+      $.cookie(stateCookieName, height, { expires: 365 });
+    }
+    
+    textarea.height(height + 'px');
   });
 });
+

trac/admin/web_ui.py

@@ -30 +30 @@
 from trac.util.text import exception_to_unicode
 from trac.util.translation import _, ngettext
 from trac.web import HTTPNotFound, IRequestHandler
-from trac.web.chrome import add_notice, add_stylesheet, \
+from trac.web.chrome import add_notice, add_stylesheet, add_script, \
                             add_warning, Chrome, INavigationContributor, \
                             ITemplateProvider
 
@@ -141 +141 @@
         })
 
         add_stylesheet(req, 'common/css/admin.css')
+        add_script(req, 'common/js/jquery_cookies.js')
+
         return template, data, None
 
     # ITemplateProvider methods

trac/wiki/web_ui.py

@@ -126 +126 @@
                   num=version, name=page.name))
 
         add_stylesheet(req, 'common/css/wiki.css')
+        add_script(req, 'common/js/jquery_cookies.js')
 
         if req.method == 'POST':
             if action == 'edit':

[Carsten Klein <carsten.klein@…>]

Still untested on IE, though. If someone would step in and test this on IE…

[Carsten Klein <carsten.klein@…>]

Works on

• FireFox 3.5.8+
• Konqueror 4.3.5+

Could someone please test

• Safari (Windows/Mac)
• IE (7/8/9)
• Chrome
• Opera >9

Thanks!

[Carsten Klein <carsten.klein@…>]

Another issue still remains: the number of cookies is restricted by some user agents, with the least denominator being 20 per domain set forth by the original HTTP specification.

So if a user has multiple plugins installed that all include templates that would render resizables, then that number can be exceeded easily, provably resulting in javascript errors.

See ​http://www.nczonline.net/blog/2009/05/05/http-cookies-explained/ for more information.

In addition to that, each cookie can only be up to 4 KiB in size.

So that leaves us with the question on how we would deal with that.

My proposal would be to either use local storage when available, as is supported by most modern user agents, or implement a wrapper to a cookie store that then would persist user interface settings so that

a) will join multiple cookies using the subcookie approach into one, and b) will distribute cookie data across multiple cookies, so that c) the maximum size of a cookie never exceeds 4 KiB, and

E.g. there exists a "virtual" cookie named "trac.ui.settings", which then is automatically distributed across multiple "trac.ui.settings.0" … "trac.ui.settings.N" cookies. When accessing trac.ui.settings and querying or setting a value in it, the cookie store will instantiate a hash from all instances of "trac.ui.settings.*", and, when persisting information again, it will automatically distribute the data across multiple "trac.ui.settings.*" instances.

For when is 12.1 being scheduled? Maybe we should move this to a later milestone?

[Remy Blank]

Another option would be to store on the server side, and to update the value on release through an XMLHttpRequest. This would also allow propagating the stored values when using different browsers / machines.

[Christian Boos]

Replying to rblank:

Another option would be to store on the server side, and to update the value on release through an XMLHttpRequest. This would also allow propagating the stored values when using different browsers / machines.

Well, are you really sure this is a good idea at all? To me, the benefit (if any) doesn't seem worth the trouble of bundling a new dependency and the extra complexity (the user settings would be spread partly in the session, partly in cookies).

[Remy Blank]

Replying to cboos:

Well, are you really sure this is a good idea at all? To me, the benefit (if any) doesn't seem worth the trouble of bundling a new dependency and the extra complexity (the user settings would be spread partly in the session, partly in cookies).

Actually, I meant dropping the cookie idea altogether and only relying on session data. Use the session data for the initial field heights on page load, and update the setting through xhr on mouse release.

[Christian Boos]

Replying to rblank:

Actually, I meant dropping the cookie idea altogether and only relying on session data. Use the session data for the initial field heights on page load,

OK

and update the setting through xhr on mouse release.

That still sounds overkill, only sending the value during a real submit (preview or save) seems good enough.

Anyway, this is not appropriate for 0.12.x, I think.

[Carsten Klein <carsten.klein@…>]

Replying to rblank:

Actually, I meant dropping the cookie idea altogether and only relying on session data. Use the session data for the initial field heights on page load, and update the setting through xhr on mouse release.

This is an excellent idea. We could make a request handler for this, e.g. SessionStateHandler which would provide an xhr api to the client for storing and, if required, also for getting session state from the server.

I will look into this during the upcoming weekend and provide a patch.

[Carsten Klein <carsten.klein@…>]

Replying to Carsten Klein <carsten.klein@…>:

Problems with such a generic service are:

• cross site scripting attacks (not that much of a problem with simple int values being stored, but in the long run one might expect other data to be stored as well, once it becomes available)
  • data spoofing attacks
  • data / script injection attacks

In order to overcome this problem, each XHR request must be guarded using a token. This token could be that of the parent form, provided that this will not invalidate the form token.

Would this be okay with you?

[Carsten Klein <carsten.klein@…>]

Replying to Carsten Klein <carsten.klein@…>:

Dunno, but is such a token available on every form in order to prevent XSS?

[Remy Blank]

Replying to Carsten Klein <carsten.klein@…>:

Dunno, but is such a token available on every form in order to prevent XSS?

Yes, it is. Look for __FORM_TOKEN. We already pass that for the automatic preview in the wiki and ticket comments.

[Carsten Klein <carsten.klein@…>]

Replying to rblank:

Replying to Carsten Klein <carsten.klein@…>:

Dunno, but is such a token available on every form in order to prevent XSS?

Yes, it is. Look for __FORM_TOKEN. We already pass that for the automatic preview in the wiki and ticket comments.

Thanks for the hint.

As far as I can see, form token handling seems ok, but I can see possible attack vectors, thus I will also revise existing form token handling.

[Carsten Klein <carsten.klein@…>]

Long time no input on my behalf. I set this to a low priority for now, since I need to learn more on behalf on both trac+xhr requests. Besides that I got carried away with cookies, so to say. Will definitely look into this in a few weeks.

[Remy Blank]

Refocusing.

[carsten.klein@…]

I say we should close this issue and instead focus on using newer technologies than cookies, for example browser integrated SQLite storages and so on.

This, of course, will break with older browser versions, but who cares?