Revision 772806.
Locked
 
Administrator
|
Following this fix, I wonder if we should not put StringUtil.wrapString around all error messages since some of them use html
entities and we don't care about security at this stage. Jacques PS : Mmm... Actually I already did it for framework at r767694. Should be extended to all message.ftl files... |
|
Which error messages are you referring to, ie coming from where? -David On May 7, 2009, at 5:23 PM, Jacques Le Roux wrote: > Following this fix, I wonder if we should not put > StringUtil.wrapString around all error messages since some of them > use html entities and we don't care about security at this stage. > > Jacques > PS : Mmm... Actually I already did it for framework at r767694. > Should be extended to all message.ftl files... > > |
|
Administrator
|
From: "David E Jones" <[hidden email]>
> Which error messages are you referring to, ie coming from where? Error and event messages (respective lists also) rendered in UI by message.ftl files At r772887, I generalized r767694 to allow better error messages rendering. Actually I was more interested to solve the same issue in BizznessTime, but StringUtil.wrapString is not working there (I guess humanMsg.displayMsg is waiting a string and not something embedded). I gave up :/ The reason I was trying to do that is that if you get a message which allows to fill a sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330, you are not able to read it in BizznessTime. And as it's the default now it's pretty annoying. As you can see in OFBIZ-2330 this stratagem as pretty well worked so far to get returns from users. Jacques PS: the kind of msg I'm reffering to contains <<Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 (check before if a sub-task for this error does not exist). If you are not sure how to create a Jira issue please have a look before at http://docs.ofbiz.org/x/r. Thank you in advance for your help.>> > -David > > > On May 7, 2009, at 5:23 PM, Jacques Le Roux wrote: > >> Following this fix, I wonder if we should not put StringUtil.wrapString around all error messages since some of them use html >> entities and we don't care about security at this stage. >> >> Jacques >> PS : Mmm... Actually I already did it for framework at r767694. Should be extended to all message.ftl files... >> >> > |
|
|
I was asking more about where the messages are coming from, not how they get displayed (all messages come through those generic messages file...). We discussed this a while back (around the time of adding the auto-escaping) and it has come up a couple of times since then. The decision so far has been to escape these messages so they appear literally to the user. Part of the reason for that is that we don't want HTML in these messages. So no, we shouldn't wrap them in a non-String to get around the HTML escaping. In other words, the messages strings should be escaped. However, if there is a specific scenario you've run into where HTML is needed in the messages, please share that and we can discuss whether or not to change the direction that has been established. I think you were trying to list one below, but I'm not really sure as that looks like just the type of messages that we would WANT to be escaped so that none of the characters in it area interpreted as tags. I can understand that maybe you forgot about these discussions, but at least wait for feedback. Unless others agree and there is a good reason to not escape error messages my opinion is that the changes in revs 772887 and 772893 should be reverted. -David On May 8, 2009, at 2:58 AM, Jacques Le Roux wrote: > From: "David E Jones" <[hidden email]> >> Which error messages are you referring to, ie coming from where? > > Error and event messages (respective lists also) rendered in UI by > message.ftl files > At r772887, I generalized r767694 to allow better error messages > rendering. > Actually I was more interested to solve the same issue in > BizznessTime, but StringUtil.wrapString is not working there (I > guess humanMsg.displayMsg is waiting a string and not something > embedded). I gave up :/ > The reason I was trying to do that is that if you get a message > which allows to fill a sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 > , you are not able to read it in BizznessTime. And as it's the > default now it's pretty annoying. As you can see in OFBIZ-2330 this > stratagem as pretty well worked so far to get returns from users. > > Jacques > PS: the kind of msg I'm reffering to contains > <<Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 > (check before if a sub-task for this error does not exist). > If you are not sure how to create a Jira issue please have a look > before at http://docs.ofbiz.org/x/r. > > Thank you in advance for your help.>> > >> -David >> >> >> On May 7, 2009, at 5:23 PM, Jacques Le Roux wrote: >> >>> Following this fix, I wonder if we should not put >>> StringUtil.wrapString around all error messages since some of >>> them use html entities and we don't care about security at this >>> stage. >>> >>> Jacques >>> PS : Mmm... Actually I already did it for framework at r767694. >>> Should be extended to all message.ftl files... >>> >>> > > |
|
Administrator
|
From: "David E Jones" <[hidden email]>
> I was asking more about where the messages are coming from, not how they get displayed (all messages come through those generic > messages file...). We discussed this a while back (around the time of adding the auto-escaping) and it has come up a couple of > times since then. > > The decision so far has been to escape these messages so they appear literally to the user. Part of the reason for that is that > we don't want HTML in these messages. > > So no, we shouldn't wrap them in a non-String to get around the HTML escaping. In other words, the messages strings should be > escaped. I agree, actually I mixed 2 problems. The one I reported at bottom of http://docs.ofbiz.org/x/JAw (New Features Roadmap - Living Document) which concerns all internationalized labels and not only error and events messages And the rendering of internationalized error and events messages by messages.ftl templates were some messages are comming from internationalized labels and some come from other parts of OFBiz (like the one at hand which comes from ServiceEventHandler.checkSecureParameter where actually I have used \n and no HTML tags) This was in my haste of solving this issue in BuzzinessTime yesterday at the last moment and I continued in a hurry without thinking more about it this morning. Actually also these problems are mixed since they both relate to HTML being embedded in errort/event messages rendered finally in message.ftl templates. I understand an I agree with a global policy, that would want that we should not put any HTML tags into internationalized labels, since these labels may be used out of the HTTP protocol. This should not be to hard to do, even if we lose some easiness for labels like ProductRedExplanation. So better than putting a wrapper around those strings (as I done also for ProductRedExplanation which is not an error or event message) we should simply remove HTML tags from all internationalized labels. Before removing all HTML tags from internationalized labels and reverting r772806, r772893, r772887, r767695, and r767694 I'd like to be sure that we all agree on this. Also Adrian suggested, following CSS style best practices, that we shouldn't name things "ProductRedExplanation" because it implies a color, but rather ProductHighlightedExplanation. I agree and could do that too in the same action. > However, if there is a specific scenario you've run into where HTML is needed in the messages, please share that and we can > discuss whether or not to change the direction that has been established. I think you were trying to list one below, but I'm not > really sure as that looks like just the type of messages that we would WANT to be escaped so that none of the characters in it > area interpreted as tags. The only problem I see is how to render new lines (to clearly separate error/event message sub-contents in a sole message) to make those messages more readable (or even readable for BizznessTime) in an HTML rendering context. Actually it's why I beginned with all that... To eventually get more informations (sub-tasks) in https://issues.apache.org/jira/browse/OFBIZ-2330 Jacques > I can understand that maybe you forgot about these discussions, but at least wait for feedback. Unless others agree and there is > a good reason to not escape error messages my opinion is that the changes in revs 772887 and 772893 should be reverted. > > -David > > > On May 8, 2009, at 2:58 AM, Jacques Le Roux wrote: > >> From: "David E Jones" <[hidden email]> >>> Which error messages are you referring to, ie coming from where? >> >> Error and event messages (respective lists also) rendered in UI by message.ftl files >> At r772887, I generalized r767694 to allow better error messages rendering. >> Actually I was more interested to solve the same issue in BizznessTime, but StringUtil.wrapString is not working there (I guess >> humanMsg.displayMsg is waiting a string and not something embedded). I gave up :/ >> The reason I was trying to do that is that if you get a message which allows to fill a sub-task of >> https://issues.apache.org/jira/browse/OFBIZ-2330 , you are not able to read it in BizznessTime. And as it's the default now it's >> pretty annoying. As you can see in OFBIZ-2330 this stratagem as pretty well worked so far to get returns from users. >> >> Jacques >> PS: the kind of msg I'm reffering to contains >> <<Moreover it would be kind if you could create a Jira sub-task of https://issues.apache.org/jira/browse/OFBIZ-2330 >> (check before if a sub-task for this error does not exist). >> If you are not sure how to create a Jira issue please have a look before at http://docs.ofbiz.org/x/r. >> >> Thank you in advance for your help.>> >> >>> -David >>> >>> >>> On May 7, 2009, at 5:23 PM, Jacques Le Roux wrote: >>> >>>> Following this fix, I wonder if we should not put StringUtil.wrapString around all error messages since some of them use >>>> html entities and we don't care about security at this stage. >>>> >>>> Jacques >>>> PS : Mmm... Actually I already did it for framework at r767694. Should be extended to all message.ftl files... >>>> >>>> >> >> > |
|
|
On May 8, 2009, at 9:27 AM, Jacques Le Roux wrote: > From: "David E Jones" <[hidden email]> >> However, if there is a specific scenario you've run into where HTML >> is needed in the messages, please share that and we can discuss >> whether or not to change the direction that has been established. >> I think you were trying to list one below, but I'm not really sure >> as that looks like just the type of messages that we would WANT to >> be escaped so that none of the characters in it area interpreted >> as tags. > > The only problem I see is how to render new lines (to clearly > separate error/event message sub-contents in a sole message) to make > those messages more readable (or even readable for BizznessTime) in > an HTML rendering context. Actually it's why I beginned with all > that... To eventually get more informations (sub-tasks) in https://issues.apache.org/jira/browse/OFBIZ-2330 The formatting should be handled by the template... that's kind of the whole point of keeping formatting out of the messages (they can mess up template formatting, and it makes them unusable for non-web purposes). If you want multiple messages use the _ERROR_MESSAGE_LIST_ instead of an _ERROR_MESSAGE_. -David |
|
Administrator
|
From: "David E Jones" <[hidden email]>
> On May 8, 2009, at 9:27 AM, Jacques Le Roux wrote: > >> From: "David E Jones" <[hidden email]> >>> However, if there is a specific scenario you've run into where HTML >>> is needed in the messages, please share that and we can discuss >>> whether or not to change the direction that has been established. >>> I think you were trying to list one below, but I'm not really sure >>> as that looks like just the type of messages that we would WANT to >>> be escaped so that none of the characters in it area interpreted >>> as tags. >> >> The only problem I see is how to render new lines (to clearly >> separate error/event message sub-contents in a sole message) to make >> those messages more readable (or even readable for BizznessTime) in >> an HTML rendering context. Actually it's why I beginned with all >> that... To eventually get more informations (sub-tasks) in https://issues.apache.org/jira/browse/OFBIZ-2330 > > The formatting should be handled by the template... that's kind of the > whole point of keeping formatting out of the messages (they can mess > up template formatting, and it makes them unusable for non-web > purposes). > > If you want multiple messages use the _ERROR_MESSAGE_LIST_ instead of > an _ERROR_MESSAGE_. OK, I gave up for this particular and temporary case. Commited at r773076 and r773081 for R9.04 I'm reverting my other changes too Jacques > -David > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |