Re: svn commit: r1746820 - /ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl

[Deepak Dixit-3]

Hi Arjun,

Its incorrect markup, form tag is not valid child for table, you can't put
form between td tag, You need to put this inside td.

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com

On Sat, Jun 4, 2016 at 6:50 PM, <[hidden email]> wrote:

> Author: pranayp
> Date: Sat Jun  4 13:20:58 2016
> New Revision: 1746820
>
> URL: http://svn.apache.org/viewvc?rev=1746820&view=rev
> Log:
> [OFBIZ-7162] Fixed security issue with delete child period in
> EditCustomTimePeriod.
>
> Thanks Montalbano Florian for reporting the issue and thanks Arjun Kaushal
> for providing the patch.
>
> Modified:
>
> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
>
> Modified:
> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl?rev=1746820&r1=1746819&r2=1746820&view=diff
>
> ==============================================================================
> ---
> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
> (original)
> +++
> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
> Sat Jun  4 13:20:58 2016
> @@ -60,7 +60,7 @@ under the License.
>            <td>${uiLabelMap.AccountingPeriodName}</td>
>            <td>${uiLabelMap.CommonFromDate}</td>
>            <td>${uiLabelMap.CommonThruDate}</td>
> -          <td>&nbsp;</td>
> +          <td colspan="2">&nbsp;</td>
>          </tr>
>            <tr>
>              <td>${currentCustomTimePeriod.customTimePeriodId}</td>
> @@ -124,12 +124,16 @@ under the License.
>              </td>
>              <td class="button-col">
>                <input type="submit" value='${uiLabelMap.CommonUpdate}'/>
> -              <a
> href='<@ofbizUrl>deleteCustomTimePeriod?customTimePeriodId=${currentCustomTimePeriod.customTimePeriodId}</@ofbizUrl>'>
> -              ${uiLabelMap.CommonDelete}</a>
> +            </td>
> +        </form>
> +            <td class="button-col">
> +              <form method="post"
> action='<@ofbizUrl>deleteCustomTimePeriod</@ofbizUrl>'
> name='deleteCustomTimePeriodForm'>
> +                <input type="hidden" name="customTimePeriodId"
> value="${currentCustomTimePeriod.customTimePeriodId!}" />
> +                <input type="submit" value='${uiLabelMap.CommonDelete}'/>
> +              </form>
>              </td>
>            </tr>
>        </table>
> -        </form>
>      <#else>
>        <div
> class="screenlet-body">${uiLabelMap.AccountingNoCurrentCustomTimePeriodSelected}</div>
>      </#if>
> @@ -152,7 +156,7 @@ under the License.
>            <td>${uiLabelMap.AccountingPeriodName}</td>
>            <td>${uiLabelMap.CommonFromDate}</td>
>            <td>${uiLabelMap.CommonThruDate}</td>
> -          <td>&nbsp;</td>
> +          <td colspan="3">&nbsp;</td>
>          </tr>
>          <#assign line = 0>
>          <#list customTimePeriods as customTimePeriod>
> @@ -213,15 +217,21 @@ under the License.
>                  <#if nowTimestamp.after(compareDate)><#assign hasExpired
> = true></#if>
>                </#if>
>                <input type="text" size='13' name="thruDate"
> value="${customTimePeriod.thruDate?string("yyyy-MM-dd")}"<#if hasExpired>
> class="alert"</#if> />
> -             </td>
> -             <td class="button-col">
> +            </td>
> +            <td class="button-col">
>                <input type="submit" value='${uiLabelMap.CommonUpdate}'/>
> -              <a
> href='<@ofbizUrl>deleteCustomTimePeriod?customTimePeriodId=${customTimePeriod.customTimePeriodId!}&amp;currentCustomTimePeriodId=${currentCustomTimePeriodId!}&amp;findOrganizationPartyId=${findOrganizationPartyId!}</@ofbizUrl>'>
> -              ${uiLabelMap.CommonDelete}</a>
> +            </td>
> +            </form>
> +            <td class="button-col">
> +              <form method="post"
> action='<@ofbizUrl>deleteCustomTimePeriod</@ofbizUrl>'
> name='lineForm${line}'>
> +                <input type="hidden" name="customTimePeriodId"
> value="${customTimePeriod.customTimePeriodId!}" />
> +                <input type="submit" value='${uiLabelMap.CommonDelete}'/>
> +              </form>
> +            </td>
> +            <td class="button-col">
>                <a
> href='<@ofbizUrl>EditCustomTimePeriod?currentCustomTimePeriodId=${customTimePeriod.customTimePeriodId!}&amp;findOrganizationPartyId=${findOrganizationPartyId!}</@ofbizUrl>'>
>                ${uiLabelMap.CommonSetAsCurrent}</a>
>              </td>
> -            </form>
>            </tr>
>          </#list>
>        </table>
>
>
>

[Jacques Le Roux]

This is right Deeak,

Moreover this is what says the "HTML Validator" plugin in Firefox (http://users.skynet.be/mgueury/mozilla/) on demo trunk (HEAD)

Result: 61 erreurs / 0 avertissements

Info: W3c Online Validation

line 286 column 49 - Erreur: The “cellspacing” attribute on the “table” element is obsolete. Use CSS instead.
line 299 column 133 - Erreur: Start tag “form” seen in “table”.
line 299 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 300 column 76 - Erreur: Start tag “input” seen in “table”.
line 300 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 394 column 19 - Erreur: Stray end tag “form”.
line 394 column 19 - Erreur: Stray end tag “form”.
line 407 column 133 - Erreur: Start tag “form” seen in “table”.
line 407 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 408 column 76 - Erreur: Start tag “input” seen in “table”.
line 408 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 502 column 19 - Erreur: Stray end tag “form”.
line 502 column 19 - Erreur: Stray end tag “form”.
line 515 column 133 - Erreur: Start tag “form” seen in “table”.
line 515 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 516 column 76 - Erreur: Start tag “input” seen in “table”.
line 516 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 610 column 19 - Erreur: Stray end tag “form”.
line 610 column 19 - Erreur: Stray end tag “form”.
line 623 column 133 - Erreur: Start tag “form” seen in “table”.
line 623 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 624 column 76 - Erreur: Start tag “input” seen in “table”.
line 624 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 718 column 19 - Erreur: Stray end tag “form”.
line 718 column 19 - Erreur: Stray end tag “form”.
line 731 column 133 - Erreur: Start tag “form” seen in “table”.
line 731 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 732 column 76 - Erreur: Start tag “input” seen in “table”.
line 732 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 826 column 19 - Erreur: Stray end tag “form”.
line 826 column 19 - Erreur: Stray end tag “form”.
line 839 column 133 - Erreur: Start tag “form” seen in “table”.
line 839 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 840 column 76 - Erreur: Start tag “input” seen in “table”.
line 840 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 934 column 19 - Erreur: Stray end tag “form”.
line 934 column 19 - Erreur: Stray end tag “form”.
line 947 column 133 - Erreur: Start tag “form” seen in “table”.
line 947 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 948 column 76 - Erreur: Start tag “input” seen in “table”.
line 948 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1042 column 19 - Erreur: Stray end tag “form”.
line 1042 column 19 - Erreur: Stray end tag “form”.
line 1055 column 133 - Erreur: Start tag “form” seen in “table”.
line 1055 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1056 column 76 - Erreur: Start tag “input” seen in “table”.
line 1056 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1150 column 19 - Erreur: Stray end tag “form”.
line 1150 column 19 - Erreur: Stray end tag “form”.
line 1163 column 133 - Erreur: Start tag “form” seen in “table”.
line 1163 column 133 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1164 column 76 - Erreur: Start tag “input” seen in “table”.
line 1164 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1258 column 19 - Erreur: Stray end tag “form”.
line 1258 column 19 - Erreur: Stray end tag “form”.
line 1271 column 134 - Erreur: Start tag “form” seen in “table”.
line 1271 column 134 - Erreur: Element “form” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1272 column 76 - Erreur: Start tag “input” seen in “table”.
line 1272 column 76 - Erreur: Element “input” not allowed as child of element “tr” in this context. (Suppressing further errors from this subtree.)
line 1366 column 19 - Erreur: Stray end tag “form”.
line 1366 column 19 - Erreur: Stray end tag “form”.

So 2 same are not from Arjun's patch. So I guess he simply followed the "trend" in this page. I guess we have still a lot like that in all OFBiz. Some
maybe introduced with subtasks of OFBIZ-2330...

I'd not call them bugs since so far browsers are accepting and rendering them. But I agree it would be good to get rid of (all of) them. This would be
another Jira ;)

Jacques

Le 06/06/2016 à 08:57, Deepak Dixit a écrit :

> Hi Arjun,
>
> Its incorrect markup, form tag is not valid child for table, you can't put
> form between td tag, You need to put this inside td.
>
> Thanks & Regards
> --
> Deepak Dixit
> www.hotwaxsystems.com
>
> On Sat, Jun 4, 2016 at 6:50 PM, <[hidden email]> wrote:
>
>> Author: pranayp
>> Date: Sat Jun  4 13:20:58 2016
>> New Revision: 1746820
>>
>> URL: http://svn.apache.org/viewvc?rev=1746820&view=rev
>> Log:
>> [OFBIZ-7162] Fixed security issue with delete child period in
>> EditCustomTimePeriod.
>>
>> Thanks Montalbano Florian for reporting the issue and thanks Arjun Kaushal
>> for providing the patch.
>>
>> Modified:
>>
>> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
>>
>> Modified:
>> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl?rev=1746820&r1=1746819&r2=1746820&view=diff
>>
>> ==============================================================================
>> ---
>> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
>> (original)
>> +++
>> ofbiz/trunk/applications/accounting/template/period/EditCustomTimePeriod.ftl
>> Sat Jun  4 13:20:58 2016
>> @@ -60,7 +60,7 @@ under the License.
>>             <td>${uiLabelMap.AccountingPeriodName}</td>
>>             <td>${uiLabelMap.CommonFromDate}</td>
>>             <td>${uiLabelMap.CommonThruDate}</td>
>> -          <td>&nbsp;</td>
>> +          <td colspan="2">&nbsp;</td>
>>           </tr>
>>             <tr>
>>               <td>${currentCustomTimePeriod.customTimePeriodId}</td>
>> @@ -124,12 +124,16 @@ under the License.
>>               </td>
>>               <td class="button-col">
>>                 <input type="submit" value='${uiLabelMap.CommonUpdate}'/>
>> -              <a
>> href='<@ofbizUrl>deleteCustomTimePeriod?customTimePeriodId=${currentCustomTimePeriod.customTimePeriodId}</@ofbizUrl>'>
>> -              ${uiLabelMap.CommonDelete}</a>
>> +            </td>
>> +        </form>
>> +            <td class="button-col">
>> +              <form method="post"
>> action='<@ofbizUrl>deleteCustomTimePeriod</@ofbizUrl>'
>> name='deleteCustomTimePeriodForm'>
>> +                <input type="hidden" name="customTimePeriodId"
>> value="${currentCustomTimePeriod.customTimePeriodId!}" />
>> +                <input type="submit" value='${uiLabelMap.CommonDelete}'/>
>> +              </form>
>>               </td>
>>             </tr>
>>         </table>
>> -        </form>
>>       <#else>
>>         <div
>> class="screenlet-body">${uiLabelMap.AccountingNoCurrentCustomTimePeriodSelected}</div>
>>       </#if>
>> @@ -152,7 +156,7 @@ under the License.
>>             <td>${uiLabelMap.AccountingPeriodName}</td>
>>             <td>${uiLabelMap.CommonFromDate}</td>
>>             <td>${uiLabelMap.CommonThruDate}</td>
>> -          <td>&nbsp;</td>
>> +          <td colspan="3">&nbsp;</td>
>>           </tr>
>>           <#assign line = 0>
>>           <#list customTimePeriods as customTimePeriod>
>> @@ -213,15 +217,21 @@ under the License.
>>                   <#if nowTimestamp.after(compareDate)><#assign hasExpired
>> = true></#if>
>>                 </#if>
>>                 <input type="text" size='13' name="thruDate"
>> value="${customTimePeriod.thruDate?string("yyyy-MM-dd")}"<#if hasExpired>
>> class="alert"</#if> />
>> -             </td>
>> -             <td class="button-col">
>> +            </td>
>> +            <td class="button-col">
>>                 <input type="submit" value='${uiLabelMap.CommonUpdate}'/>
>> -              <a
>> href='<@ofbizUrl>deleteCustomTimePeriod?customTimePeriodId=${customTimePeriod.customTimePeriodId!}&amp;currentCustomTimePeriodId=${currentCustomTimePeriodId!}&amp;findOrganizationPartyId=${findOrganizationPartyId!}</@ofbizUrl>'>
>> -              ${uiLabelMap.CommonDelete}</a>
>> +            </td>
>> +            </form>
>> +            <td class="button-col">
>> +              <form method="post"
>> action='<@ofbizUrl>deleteCustomTimePeriod</@ofbizUrl>'
>> name='lineForm${line}'>
>> +                <input type="hidden" name="customTimePeriodId"
>> value="${customTimePeriod.customTimePeriodId!}" />
>> +                <input type="submit" value='${uiLabelMap.CommonDelete}'/>
>> +              </form>
>> +            </td>
>> +            <td class="button-col">
>>                 <a
>> href='<@ofbizUrl>EditCustomTimePeriod?currentCustomTimePeriodId=${customTimePeriod.customTimePeriodId!}&amp;findOrganizationPartyId=${findOrganizationPartyId!}</@ofbizUrl>'>
>>                 ${uiLabelMap.CommonSetAsCurrent}</a>
>>               </td>
>> -            </form>
>>             </tr>
>>           </#list>
>>         </table>
>>
>>
>>

[Jacques Le Roux]

Le 06/06/2016 à 20:57, Jacques Le Roux a écrit :
> This is right Deeak,
Sorry Deepak!

Jacques

[Jacques Le Roux]

Le 06/06/2016 à 20:57, Jacques Le Roux a écrit :
> So 2 same are not from Arjun's patch. So I guess he simply followed the "trend" in this page. I guess we have still a lot like that in all OFBiz.
> Some maybe introduced with subtasks of OFBIZ-2330...
I was maybe too fast on that, I checked 2 subtasks of OFBIZ-2330 and found nothing like that, remain 80- subtasks to check and certainly more in the
wide ;)
Actually we all know that using tables for layout is not a good thing, but most of OFBiz dates from 2001 to 2010...

Jacques

[Deepak Dixit-3]

No Problem Jacques :)

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com

On Tue, Jun 7, 2016 at 12:31 AM, Jacques Le Roux <
[hidden email]> wrote:

> Le 06/06/2016 à 20:57, Jacques Le Roux a écrit :
>
>> This is right Deeak,
>>
> Sorry Deepak!
>
> Jacques
>
>