| Issue 155: | worksheet - encoding and layout | |
| 2 people starred this issue and may be notified of changes. | Back to list |
I've come across two issues in the JMesa worksheet. The first one concerns using Javascripts special characters in the input tag. You can reproduce this by trying to save a double quote character in the presidents example. I have implemented a working solution that also applies to the drop down filters by using javascripts escape() and unescape() functions on the client side, and java.net.URLDecoder#decode() and java.net.URLEncoder#encode() on the server side. The second issue can occur when there is a long value in a cell (see screenshot_long_value.png). I've used style="width: 98%;" in the div and input tags. While this solution isn't perfect, it at least prevents the input to "collide" with the next column.
Oct 14, 2008
I started looking at this tonight. Overall I am very impressed! I can tell you really dug into the API to find the points that needed better encoding. I found one problem with the droplist filter though. I am including a screenshot to show you what it looks like right now.
Oct 16, 2008
Thanks for the feedback. To handle the ' ' and '+' characters correctly, I borrowed some code from commons-codec and changed it a bit. Now the drop list filter should work properly. For this I added a class org.jmesa.util.EncodingUtils, which I use in LimitActionFactoryImpl, DroplistFilterEditor and WorksheetUpdaterImpl. In addition, I found that I also needed some html escaping. I added a class org.jmesa.view.editor.HtmlCellEditor and did a minor change in org.jmesa.view.html.editor.HtmlFilterEditor. I found an article (http://xkr.us/articles/javascript/encode-compare/) that explained the differences between javascripts escape(), encodeURI() and encodeURIComponent() functions. It helped for my understanding, but I still feel uncertain that the changes I've made will handle any situation correctly, although the changes seem to have solved my problems. In the jquery.jmesa.js, changes were made in the following functions: createHiddenInputFields, createParameterString, createDynFilter, createDroplistDynFilter, createWsColumn, submitWsColumn. And I added: String.prototype.escapeHTML
Oct 16, 2008
Thank you! I am going to look over this this weekend and then I'll get back to you...
Oct 17, 2008
>> I borrowed some code from commons-codec and changed it a bit I wonder what this means from a license standpoint...I imagine I need to give some sort of credit to the place we borrowed the code from. I have your files imported now, but have not checked it in yet. I did find a bug with the droplist but I can look into it. If everything goes well I plan on checking it into the trunk on Saturday. Are you running off the trunk now? Or, a better question is once the changes get done would you be able to run off of a jar I build for you for more testing?
Oct 18, 2008
I think I got it now :). I ended up backing up and applying your changes slowly to understand them. I think I found a few better ways to approach what you did in a way that is simplier and still has the same meaning. You still did most of the work in finding almost all the places that needed to be fixed. What I found was that the jQuery .text() method will automatically escape values like we need. I also applied the encodeURIComponent() to anything that gets submitted, but then use the URLDecoder.decode to decode the values. Lastly, in the droplist I first escape the html, and then the javascript with escapeJavaScript(escapeHtml(option.getValue())); Other than that I used most of what you submitted. So great work! I can't believe this hasn't come up before... What is the best way for you to help me test this? Are you working off the trunk, or should I send you a new jar file?
Status:
Started
Oct 20, 2008
I checked out from trunk and made the modifications from there. I've made a local "branch" to keep track of the changes I made, and have just updated the latest commits you've made. You've done some tidying up :) Nice! I've done some quick testing, and as you've mentioned, the droplist doesn't work. I'll test some more tomorrow and get back to you. PS! The pom.xml dependencies are not in sync with the ones in ivy.xml. It also seems that some jars are missing in the repo (http://jmesa.googlecode.com/svn/repository). From groovy build: - module not found: [ bsf | bsf | 2.4.0 ] - module not found: [ commons-el | commons-el | 1.0 ] - module not found: [ rhino | js | 1.7R1 ]
Oct 20, 2008
I forgot to check in those other jars...there are checked in now! Also, the Maven pom.xml file is maintained by the community, but it has gotten out of date now. I was getting some help with it recently but have not heard back from that person for a while now. https://code.google.com/p/jmesa/issues/detail?id=107 The way I do builds is with Groovy/Ant/Ivy. https://code.google.com/p/jmesa/wiki/ProjectBuild
Oct 21, 2008
1. Input style (see input_style.png):
I've tested with both Firefox 2 and IE7. Under some circumstances, when clicking a
table element, the worksheet input area overlaps with the next elements. It occurs
when the element data's size is greater than the column headers'.
I've tried removing the absolute positioning of .jmesa #wsColumnDiv:
.jmesa #wsColumnDiv {
/*position: absolute;
top: -2px;
left: -2px;
padding: 0;
margin: 0;*/
height: 17px;
border: 1px solid #c0c0c0;
background-color: #ededed;
}
That seems to do the trick.
2. Droplist:
a) The changedValue needs to be fetched with the text() function:
var changedValue = $("#dynFilterDroplistDiv option:selected").text();
b) The changedValue needs to be encoded before it is sent:
$.jmesa.addFilterToLimit(dynFilter.id, dynFilter.property,
encodeURIComponent(changedValue));
Try saving "+" and filter on it.
c) It seems that in spite that the html is escaped when building the drop list:
String value = escapeJavaScript(escapeHtml(option.getValue()));
The value still needs to be escaped on the client side:
html += '<option value="' + key + '">' + value.escapeHTML() + '</option>';
Try saving "<null>" and filter on it.
Oct 21, 2008
I will not have time to work with this tonight, but will in the next couple days. Its great to get this really ironed out finally.
Oct 22, 2008
Let me take these one at a time: 1. The CSS is necessary for the way it is currently implemented. I wonder if instead of having CSS keep the position on the screen we use jQuery to give us the position. jQuery now has the ability to tell the position on the screen and I am going to explore using that. The absolute versus relative positioning doesn't seem to be as accurate as just taking the div all the way out of the page and positioning it where we want. This would also be a much better way to do the droplist as well. At work we started doing this for other things and it seems to be a much better way to go. 2 a. The .text() pulls the option label, but we want the option value. If you want what is in the option label to be filtered you need to set the option value to the same thing. This is the default as well. 2 b. I am not seeing this behavior with the code in the trunk and running the examples. I am able to save a + and filter on it. It also shows up in the droplist fine. 2 c. I see what you mean. The problem is that the options are built in a string and then added with div.html(html);. Maybe what we need to do instead is add the droplist (select element) and then populated it. That should work much better. Can you get back to me if you are still seeing the behavior of 2b with the trunk code? I am going to start working on droplist next, and also the div positioning.
Oct 29, 2008
I know we still have the issue with the div layers. I still plan on trying the other way to work with the div layer sizing...I just got caught up in other things. Its still on my list though.
Dec 6, 2008
I think the worksheet is working pretty good now. If there are still issues outstanding we should open another ticket.
Status:
Fixed
|
80.0 KB Download