http://ofbiz.116.s1.nabble.com/Re-svn-commit-r928451-ofbiz-trunk-framework-widget-templates-htmlScreenMacroLibrary-ftl-tp1694238p1694286.html
> Adrian Crum wrote:
> > --- On Sun, 3/28/10, Adam Heath <
[hidden email]>
> wrote:
> >> Adrian Crum wrote:
> >>> --- On Sun, 3/28/10, Adam Heath <
[hidden email]>
> >> wrote:
> >>>>
[hidden email]
> >>>> 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.