--- On Sun, 3/28/10, Adam Heath <doo...@brainfood.com> wrote: > Adrian Crum wrote: > > --- On Sun, 3/28/10, Adam Heath <doo...@brainfood.com> > wrote: > >> Adrian Crum wrote: > >>> --- On Sun, 3/28/10, Adam Heath <doo...@brainfood.com> > >> wrote: > >>>> adri...@apache.org > >>>> wrote: > >>>>> Author: adrianc > >>>>> Date: Sun Mar 28 16:48:22 2010 > >>>>> New Revision: 928451 > >>>>> > >>>>> URL: http://svn.apache.org/viewvc?rev=928451&view=rev > >>>>> Log: > >>>>> Simplified label html renderer macro. > >>>>> > >>>>> Modified: > >>>>> > >>>> > >> > ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl > >>>>> Modified: > >> > ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl > >>>>> URL: > >>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl?rev=928451&r1=928450&r2=928451&view=diff > >>>>> > >> > ============================================================================== > >>>>> --- > >> > ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl > >>>> (original) > >>>>> +++ > >> > ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl > >>>> Sun Mar 28 16:48:22 2010 > >>>>> @@ -66,47 +66,10 @@ under the > License. > >>>>> <#if > >> text?has_content> > >>>>> <#-- > Label > >> is considered block > >>>> level element in screen widget. There is > not > >> reason to > >>>> render text outside of any html element. > Use of > >> style > >>>> element has set pattern and we'll use > style > >>>>> > to > >> determine > >>>> appropriate html element to use --> > >>>>> - <#if > style?has_content> > >>>>> - <#if > style=="h1"> > >>>>> - <h1 > >>>>> - <#elseif > >> style=="h2"> > >>>>> - <h2 > >>>>> - <#elseif > >> style=="h3"> > >>>>> - <h3 > >>>>> - <#elseif > >> style=="h4"> > >>>>> - <h4 > >>>>> - <#elseif > >> style=="h5"> > >>>>> - <h5 > >>>>> - <#elseif > >> style=="h6"> > >>>>> - <h6 > >>>>> - <#else> > >>>>> - <p > >> class="${style}" > >>>>> - </#if> > >>>>> + <#if > >>>> "h1~h2~h3~h4~h5~h6"?contains(style)> > >>>>> + > <${style}<#if > >>>> id?has_content> > >>>> > >> > id="${id}"</#if>>${text}</${style}> > >>>> And if style="arch1"? > >>> + <p<#if > >> style?has_content> > class="${style}"</#if><#if > >> id?has_content> > >> id="${id}"</#if>>${text}</p> > >> > >> You aren't making any sense. > >> > >> Based on what I see this change doing, we'll end > up with > >> with: > >> > >> <arch1 > >> > >> as the tag name, when style="arch1". > > > > Unless you try it out on your local copy, you will > just have to trust me that it works. :-) > > That's not a useful response. "Just trust me" is not > helpful. If you > know why something will work, and someone obvious > doesn't(me), then > just go ahead and explain it. Doing a bunch of > hand-waving just gives > the hand-waver a feeling of superiority, and makes the > receiver feel > inferior. > > However, upon further reflection, my initial example of > wrongness was > not correct. Consider when style="h1~h2". The > original code would do > <p class="h1~h2">, which could be matched in css as > p."h1~h2" { }. > > Now, with this change, the tag will be <h1~h2>, which > is very wrong. > > The question to ask yourself, is: are the widget systems, > and their > internal helper files, supposed to be general purpose, or > are they > just to be used by ofbiz? If the latter, then we can > do whatever we > want with not checking various parameters, as we can just > fix all bad > uses. > > However, if the former is the correct answer, then we have > to handle, > and allow, all possible cases. This change keeps that > from occuring.
Checking for the h* styles is OFBiz-specific behavior. You are correct that odd class names will trick the code - but the chances that someone would create a "h1~h2" class are pretty small. In any event, I will revert it.
