Stripes

Add support ala Struts to generate HTML or XHTML compliant close tags

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: Release 1.5
  • Fix Version/s: Release 1.5.5
  • Component/s: Tag Library
  • Labels:
    None
  • Environment:
    No specific OS required; no specific Java version required; etc...

Description

HTML and XHTML documents have some key differences.

For example - if we consider the <input> tag:

  • In HTML, the <input> tag has no end tag e.g. <input name="website.url" type="text" size="30">
  • In XHTML, the <input> tag must be properly closed, like this <input /> e.g. <input name="website.url" type="text" size="30" />

Stripes 1.5.x however does not have a mechanism to discern whether or not to properly close tags or not and as such takes the safer approach which is to explicitly close tags as it results in valid XHTML and is not an error for HTML but results in a warning when validating HTML documents. Although the latter is not a critical issue it does result in needless or unnecessary complaints when validating and as such is an annoyance albeit minor.

Struts since 1.x has solved this issue quite easily by allowing the inclusion of the xhtml="true" attribute to mark that closure is required (false indicates no closure). In this manner authors of XHTML and HTML documents are equally satisfied in not having any errors or extraneous warnings.

Timothy Stone had reported this issue and classified it as a bug here:
http://www.stripesframework.org/jira/browse/STS-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11931#action_11931

The issue was rightly closed as "Not a bug" as most of the discussion was based on a non-w3c validator which yielded results that considered the validation unsuccessful which is not the case with the w3c validator (not to mention that it is irrelevant whether or not XHTML is considered dead or we should align to HTML 5 - etc...).

As such this issue report is a re-statement of the above closed issue reported as an improvement and setting the stage for patch to be attached.

  1. stripes_html_mode-1.5.5_1.patch
    11/Nov/10 11:53 AM
    17 kB
    Timothy Stone
  2. stripes_html_mode-1.5.5_2.patch
    18/Nov/10 7:54 PM
    15 kB
    Timothy Stone
  3. stripes_html_mode-1.5.5_3.patch
    23/Nov/10 10:06 AM
    9 kB
    Ben Gunter
  4. stripes-html-config.tgz
    09/Nov/10 2:18 PM
    3 kB
    Timothy Stone
  5. stripes-xhtml-patch.tar.gz
    30/May/10 8:52 AM
    2 kB
    Timothy Stone
  6. stripes-xhtml-patches.tar.gz
    28/May/10 2:41 PM
    2 kB
    Nikolaos

Activity

Hide
Nikolaos added a comment - 28/May/10 2:41 PM

Attaching patch bundle that Timothy Stone authored.

(Ben you and I received the bundle from Timothy 2 days ago directly via e-mail - I looked it over and the changes look very minimal and seem correct however I have yet to test; Timothy I went ahead and re-stated the issue as an improvement and attached your code to expedite this patch - in addition attachments can not be added to a closed issue).

Hope this can still make it into the 1.5.4 release. Thanks Ben and Timothy.

Show
Nikolaos added a comment - 28/May/10 2:41 PM Attaching patch bundle that Timothy Stone authored. (Ben you and I received the bundle from Timothy 2 days ago directly via e-mail - I looked it over and the changes look very minimal and seem correct however I have yet to test; Timothy I went ahead and re-stated the issue as an improvement and attached your code to expedite this patch - in addition attachments can not be added to a closed issue). Hope this can still make it into the 1.5.4 release. Thanks Ben and Timothy.
Nikolaos made changes - 28/May/10 2:41 PM
Field Original Value New Value
Attachment stripes-xhtml-patches.tar.gz [ 10313 ]
Hide
Nikolaos added a comment - 28/May/10 2:45 PM

I should also add that Timothy's patch implicitly assumes "xhtml=true" which makes it backwards compatible with Stripes 1.5.x.
i.e. Developers who don't leverage the attribute (aka existing apps) will see no change in the output produced by Stripes....

Show
Nikolaos added a comment - 28/May/10 2:45 PM I should also add that Timothy's patch implicitly assumes "xhtml=true" which makes it backwards compatible with Stripes 1.5.x. i.e. Developers who don't leverage the attribute (aka existing apps) will see no change in the output produced by Stripes....
Hide
Nikolaos added a comment - 28/May/10 2:48 PM

I accidentally set the Priority to Major when creating the issue... can someone please adjust to Minor... sorry!

Show
Nikolaos added a comment - 28/May/10 2:48 PM I accidentally set the Priority to Major when creating the issue... can someone please adjust to Minor... sorry!
Hide
Frederic Daoud added a comment - 28/May/10 3:12 PM

Priority changed to minor as requested.

Show
Frederic Daoud added a comment - 28/May/10 3:12 PM Priority changed to minor as requested.
Frederic Daoud made changes - 28/May/10 3:12 PM
Priority Major [ 3 ] Minor [ 4 ]
Hide
Timothy Stone added a comment - 28/May/10 3:28 PM

The attached patch has a NPE when attempting to resolve the parent tag of nested layout-components.

I have completely refactored the patch based on the methodology of Struts. I will be resubmitting. DO NOT USE the patch attached "28/May/10 02:41 PM"; the patch also has a MD5 checksum of:

tstone@jagdtiger [~/Downloads] % openssl md5 stripes-xhtml-patches.tar.gz
MD5(stripes-xhtml-patches.tar.gz)= 421c00acfac8a69869ccbc35974b400e

Show
Timothy Stone added a comment - 28/May/10 3:28 PM The attached patch has a NPE when attempting to resolve the parent tag of nested layout-components. I have completely refactored the patch based on the methodology of Struts. I will be resubmitting. DO NOT USE the patch attached "28/May/10 02:41 PM"; the patch also has a MD5 checksum of: tstone@jagdtiger [~/Downloads] % openssl md5 stripes-xhtml-patches.tar.gz MD5(stripes-xhtml-patches.tar.gz)= 421c00acfac8a69869ccbc35974b400e
Hide
Timothy Stone added a comment - 30/May/10 8:52 AM

This patch is a full refactoring and corrects a nasty NPE in the previously attached patch.

The methodology of the patch follows that XHTML syntax for "singleton," or empty, elements, e.g., input, remains in place. This is the current behavior of Stripes and maintains backward compatibility.

The LayoutDefinitionTag and TLD have been modified to accept a new boolean attribute named "xhtml". The default is "true." Setting the attribute to "false" will render empty elements without the terminating "space-slash-gt" (" />").

However, while correcting the NPE, nested Layout Components do not see the PageContext of the LayoutDefinition and cannot see the xhtml flag, true or false. This can be corrected, however, I am just becoming familiar with the source code. Maybe someone can run with this as a foundation.

Show
Timothy Stone added a comment - 30/May/10 8:52 AM This patch is a full refactoring and corrects a nasty NPE in the previously attached patch. The methodology of the patch follows that XHTML syntax for "singleton," or empty, elements, e.g., input, remains in place. This is the current behavior of Stripes and maintains backward compatibility. The LayoutDefinitionTag and TLD have been modified to accept a new boolean attribute named "xhtml". The default is "true." Setting the attribute to "false" will render empty elements without the terminating "space-slash-gt" (" />"). However, while correcting the NPE, nested Layout Components do not see the PageContext of the LayoutDefinition and cannot see the xhtml flag, true or false. This can be corrected, however, I am just becoming familiar with the source code. Maybe someone can run with this as a foundation.
Timothy Stone made changes - 30/May/10 8:52 AM
Attachment stripes-xhtml-patch.tar.gz [ 10314 ]
Hide
Nikolaos added a comment - 30/May/10 10:55 AM

Timothy Stone said:

> However, while correcting the NPE, nested Layout Components do not see the PageContext of the LayoutDefinition and cannot see the xhtml flag, true or false. This > can be corrected, however, I am just becoming familiar with the source code. Maybe someone can run with this as a foundation.

The problem you describe is fortunately not a Stripes issue but a Java implementation issue. In Java here is PAGE, REQUEST, SESSION and APPLICATION scopes and each of the respective scopes I listed expands its reach over the previous one. So instead of PAGE scope the next scope up is REQUEST and that will allow any variables set to be available for the life of the "request" (not for the life of the "page" which you have now) which is exactly what you need in this case as layouts and renders appear on separate pages but are all part of the same request.

The solution is trivial... simply replace all of your occurrences of: PageContext.PAGE_SCOPE with: PageContext.REQUEST_SCOPE

That "should" solve the issue you describe. As you are all wired up with your patch code I think it logically makes sense for you to easily make the change, test and submit a revised working patch to facilitate integration by the Stripes development team. Having someone else run with this will quite likely eliminate it from 1.5.4... .

Show
Nikolaos added a comment - 30/May/10 10:55 AM Timothy Stone said: > However, while correcting the NPE, nested Layout Components do not see the PageContext of the LayoutDefinition and cannot see the xhtml flag, true or false. This > can be corrected, however, I am just becoming familiar with the source code. Maybe someone can run with this as a foundation. The problem you describe is fortunately not a Stripes issue but a Java implementation issue. In Java here is PAGE, REQUEST, SESSION and APPLICATION scopes and each of the respective scopes I listed expands its reach over the previous one. So instead of PAGE scope the next scope up is REQUEST and that will allow any variables set to be available for the life of the "request" (not for the life of the "page" which you have now) which is exactly what you need in this case as layouts and renders appear on separate pages but are all part of the same request. The solution is trivial... simply replace all of your occurrences of: PageContext.PAGE_SCOPE with: PageContext.REQUEST_SCOPE That "should" solve the issue you describe. As you are all wired up with your patch code I think it logically makes sense for you to easily make the change, test and submit a revised working patch to facilitate integration by the Stripes development team. Having someone else run with this will quite likely eliminate it from 1.5.4... .
Hide
Timothy Stone added a comment - 30/May/10 3:15 PM

Thanks Nikolaos!

I've coded the change. Looking at the test cases in the source, there does not seem to be one that exercises the tag packages. I can test these in a real application however starting Tuesday.

Also, any chance you or Freddy can delete the original 5/28/2010 patch bundle? I don't want it to accidentally leak into the wild. It has a NPE.

Show
Timothy Stone added a comment - 30/May/10 3:15 PM Thanks Nikolaos! I've coded the change. Looking at the test cases in the source, there does not seem to be one that exercises the tag packages. I can test these in a real application however starting Tuesday. Also, any chance you or Freddy can delete the original 5/28/2010 patch bundle? I don't want it to accidentally leak into the wild. It has a NPE.
Hide
Timothy Stone added a comment - 02/Jun/10 10:07 PM

Alright, I need some help... generally, the patch works. However, without using PageContent.APPLICATION_SCOPE or SESSION_SCOPE, the PAGE and REQUEST scopes are not providing the nested tags with the attribute set on the layout-definition.

It might help to know how my brain was working in thinking about this enhancement.

A new attribute would be provided to the LayoutDefinitionTag: xhtml="(true|false)". The attribute would default to "true" to support current behavior and the TLD made the attribute optional. For example:

<s:layout-definition xhtml="false">

The LayoutDefinitionTag was chosen for at least two reasons: 1) less frequency of declaration than in components or render tags and 2) it followed the Struts <html:html xhtml="false"> pattern of the top most tag.

I did not want to drop right into the use of the Session and Application scopes for the following reason: a tag attribute placed in the Application scope would be better defined in the web.xml; Session scope fells tied to a user/client experience more than a Page or response to a Request.

In looking at the StripesLayoutTag class there are a number of possible other tools to use that might provide the attribute with scope to components and renderers, but I'm not precisely sure how to use them right now (give me a few more days, maybe).

If someone were review the patch, provide a pointer, or point out my glaring n00b error, it would go a long way in my growth as a Stripes user/developer.

Much thanks.

Show
Timothy Stone added a comment - 02/Jun/10 10:07 PM Alright, I need some help... generally, the patch works. However, without using PageContent.APPLICATION_SCOPE or SESSION_SCOPE, the PAGE and REQUEST scopes are not providing the nested tags with the attribute set on the layout-definition. It might help to know how my brain was working in thinking about this enhancement. A new attribute would be provided to the LayoutDefinitionTag: xhtml="(true|false)". The attribute would default to "true" to support current behavior and the TLD made the attribute optional. For example: <s:layout-definition xhtml="false"> The LayoutDefinitionTag was chosen for at least two reasons: 1) less frequency of declaration than in components or render tags and 2) it followed the Struts <html:html xhtml="false"> pattern of the top most tag. I did not want to drop right into the use of the Session and Application scopes for the following reason: a tag attribute placed in the Application scope would be better defined in the web.xml; Session scope fells tied to a user/client experience more than a Page or response to a Request. In looking at the StripesLayoutTag class there are a number of possible other tools to use that might provide the attribute with scope to components and renderers, but I'm not precisely sure how to use them right now (give me a few more days, maybe). If someone were review the patch, provide a pointer, or point out my glaring n00b error, it would go a long way in my growth as a Stripes user/developer. Much thanks.
Hide
Iwao AVE! added a comment - 03/Jun/10 3:23 AM

Unless the changes are extremely small and simple, I would vote -1 for the change.

I've been reading the series of comments, but tweaking the framework just for avoiding occasional HTML validator warnings seems to be too much to me.
On the other hand, it should be pretty easy and legitimate for you to build HTML 4 compatible version of Stripes locally.

  1. And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so?
Show
Iwao AVE! added a comment - 03/Jun/10 3:23 AM Unless the changes are extremely small and simple, I would vote -1 for the change. I've been reading the series of comments, but tweaking the framework just for avoiding occasional HTML validator warnings seems to be too much to me. On the other hand, it should be pretty easy and legitimate for you to build HTML 4 compatible version of Stripes locally.
  1. And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so?
Hide
Timothy Stone added a comment - 03/Jun/10 8:50 AM

I voted for this issue to cancel any future vote by Iwao AVE!.

As Iwao is regular voice heard on the mailing list and I'm a n00b with a narrowly focused agenda (or so it may seem to the Java engineer with a casual exposure to HTML), I'll keep plugging at the issue in some spare cycles. Know that my solicitation for help was sincere and the focus of my fix is a little more than the "occasional HTML validator warning."

Iwao's suggestion to locally fork, while both extremely trivial and an option for users of Stripes in small or medium business projects, it is not an option for large, international organizations with governance and other regulatory oversight on OSS use.

Iwao's comment: "And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so? " is more than a little disheartening. Stripes should be winning the hearts and minds of not just MVC framework decision makers, but the HTML developers using the tags.

Nikolaos made some important arguments in STS-556. I have since realized my position did not cover the scope of the problem. Moreover, I failed to clearly illustrate the issue and relied on readers here to investigate the standards and trends, beyond validation tools on the internet. I pulled back from the debate in STS-556, and with this comment, realized I should not have.

Show
Timothy Stone added a comment - 03/Jun/10 8:50 AM I voted for this issue to cancel any future vote by Iwao AVE!. As Iwao is regular voice heard on the mailing list and I'm a n00b with a narrowly focused agenda (or so it may seem to the Java engineer with a casual exposure to HTML), I'll keep plugging at the issue in some spare cycles. Know that my solicitation for help was sincere and the focus of my fix is a little more than the "occasional HTML validator warning." Iwao's suggestion to locally fork, while both extremely trivial and an option for users of Stripes in small or medium business projects, it is not an option for large, international organizations with governance and other regulatory oversight on OSS use. Iwao's comment: "And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so? " is more than a little disheartening. Stripes should be winning the hearts and minds of not just MVC framework decision makers, but the HTML developers using the tags. Nikolaos made some important arguments in STS-556. I have since realized my position did not cover the scope of the problem. Moreover, I failed to clearly illustrate the issue and relied on readers here to investigate the standards and trends, beyond validation tools on the internet. I pulled back from the debate in STS-556, and with this comment, realized I should not have.
Hide
Iwao AVE! added a comment - 03/Jun/10 10:41 AM

Timothy,

I apologize if my last comment sounded like insulting.
That must be because of my poor English skills and nothing more than that (I had been editing the comment since last week trying to make it 'lighter' somehow, but failed apparently... it's a shame).
Any contribution by you or Nikolaos (or anybody) should be respected and I really do.

I also understand that the decision had been made as a result of IRC discussion and I always respect the community's decision.
I just thought it's better to leave a comment expressing different opinion as there seemed to be no one disagreeing.
Like you, I am just another user who wants to make Stripes better, sometimes by avoiding complexity.


I still don't get your explanation about the local modification part.
Using OSS is OK, but using customized OSS is treated like a security risks or something?
Sorry for my ignorance, but it doesn't make sense to me.

Regards,
Iwao

Show
Iwao AVE! added a comment - 03/Jun/10 10:41 AM Timothy, I apologize if my last comment sounded like insulting. That must be because of my poor English skills and nothing more than that (I had been editing the comment since last week trying to make it 'lighter' somehow, but failed apparently... it's a shame). Any contribution by you or Nikolaos (or anybody) should be respected and I really do. I also understand that the decision had been made as a result of IRC discussion and I always respect the community's decision. I just thought it's better to leave a comment expressing different opinion as there seemed to be no one disagreeing. Like you, I am just another user who wants to make Stripes better, sometimes by avoiding complexity. – I still don't get your explanation about the local modification part. Using OSS is OK, but using customized OSS is treated like a security risks or something? Sorry for my ignorance, but it doesn't make sense to me. Regards, Iwao
Hide
Nikolaos added a comment - 03/Jun/10 11:30 AM

>> Unless the changes are extremely small and simple, I would vote -1 for the change.

Iwao - the patch IS extremely small and simple... I mean how much more basic can you get when it comes to a Tag enhancement than adding a boolean at the top of something (that defaults to true) and that is read below in a simple if condition doing ">" or "/>". Please look at the submission code if you are concerned with de-stabalizing Stripes as I think far more things are likely... like getting sucked up in a huge sink hole.

The fact that it doesn't work is the problem and the fact that few people have the cycles to make it work is the issue. Also the patch should not have been submitted IMO until / unless it was thoroughly tested and working. But those are side issues to the point of providing this enhancement.

>> I've been reading the series of comments, but tweaking the framework just for avoiding occasional HTML validator warnings seems to be too much to me.

Correction... it is NOT the "occasional" warning... the warning occurs 100% of the time with HTML 4.01 validation. Sure... the severity of this issue is debatable as is whether or not to do it... BUT the issue is clear... and also that is why it is a Minor issue and marked as an Improvement NOT a bug (the original was marked as a bug which I personally didn't agree with).

If the Stripes development team feels that the code - once it is working - is not worth integrating for whatever reason that is up to them... but lets get the facts right.

>> On the other hand, it should be pretty easy and legitimate for you to build HTML 4 compatible version of Stripes locally.

This argument is silly... . If software was built that way I doubt you would have Stripes or any other OS framework. It's a framework for a reason. Please go back and re-read the comments. I think you see this as bigger than it is...

Think about a world were every improvement you suggest gets an answer like... we'll if you want it... fork the code... and the best part: maintain and patch every next release of the code... and BTW keep doing this with anywhere from 1 today to a dozen or more tomorrow... we just don't build software that way... .

I come from a large project java software development background - forked code doesn't make it into the Enterprise - period.

>> # And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so?

Okay... equally silly argument. Come on... I see the sarcasm... but there is a saying that says "with every little joke there is a lit bit of truth"... I really hope your 100% joking on this but others may not think so. In the end, the original issue was reported as a bug - something I think we all agree was incorrect - has been more recently reported as a Minor Improvement and assuming the code can get to a working state, is acceptable to integrate, and is adequately tested by the community I don't see how anyone could have an issue.

Lastly, I wouldn't knock people going elsewhere and not using Stripes when that is the #1 CON to using Stripes and trust me that is more detrimental to the long term viability of the Stripes framework than this fix is... so careful what you ask for... because you may just get it and not be happy with the outcome... a dead framework i.e. I don't think ANYONE in the Stripes community can afford turning anyone away to another framework IMO....

Show
Nikolaos added a comment - 03/Jun/10 11:30 AM >> Unless the changes are extremely small and simple, I would vote -1 for the change. Iwao - the patch IS extremely small and simple... I mean how much more basic can you get when it comes to a Tag enhancement than adding a boolean at the top of something (that defaults to true) and that is read below in a simple if condition doing ">" or "/>". Please look at the submission code if you are concerned with de-stabalizing Stripes as I think far more things are likely... like getting sucked up in a huge sink hole. The fact that it doesn't work is the problem and the fact that few people have the cycles to make it work is the issue. Also the patch should not have been submitted IMO until / unless it was thoroughly tested and working. But those are side issues to the point of providing this enhancement. >> I've been reading the series of comments, but tweaking the framework just for avoiding occasional HTML validator warnings seems to be too much to me. Correction... it is NOT the "occasional" warning... the warning occurs 100% of the time with HTML 4.01 validation. Sure... the severity of this issue is debatable as is whether or not to do it... BUT the issue is clear... and also that is why it is a Minor issue and marked as an Improvement NOT a bug (the original was marked as a bug which I personally didn't agree with). If the Stripes development team feels that the code - once it is working - is not worth integrating for whatever reason that is up to them... but lets get the facts right. >> On the other hand, it should be pretty easy and legitimate for you to build HTML 4 compatible version of Stripes locally. This argument is silly... . If software was built that way I doubt you would have Stripes or any other OS framework. It's a framework for a reason. Please go back and re-read the comments. I think you see this as bigger than it is... Think about a world were every improvement you suggest gets an answer like... we'll if you want it... fork the code... and the best part: maintain and patch every next release of the code... and BTW keep doing this with anywhere from 1 today to a dozen or more tomorrow... we just don't build software that way... . I come from a large project java software development background - forked code doesn't make it into the Enterprise - period. >> # And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so? Okay... equally silly argument. Come on... I see the sarcasm... but there is a saying that says "with every little joke there is a lit bit of truth"... I really hope your 100% joking on this but others may not think so. In the end, the original issue was reported as a bug - something I think we all agree was incorrect - has been more recently reported as a Minor Improvement and assuming the code can get to a working state, is acceptable to integrate, and is adequately tested by the community I don't see how anyone could have an issue. Lastly, I wouldn't knock people going elsewhere and not using Stripes when that is the #1 CON to using Stripes and trust me that is more detrimental to the long term viability of the Stripes framework than this fix is... so careful what you ask for... because you may just get it and not be happy with the outcome... a dead framework i.e. I don't think ANYONE in the Stripes community can afford turning anyone away to another framework IMO....
Hide
Timothy Stone added a comment - 03/Jun/10 11:46 AM

No need to apologize. Thanks for replying.

As a Stripes advocate, the technical lead of a team of HTML developers, and the internal standards creator for HTML, JS and CSS coding, I'm not without my passion regarding this improvement.

> I still don't get your explanation about the local modification part.
> Using OSS is OK, but using customized OSS is treated like a security risks or something?
> Sorry for my ignorance, but it doesn't make sense to me.

My rather large, international organization endorses the use of OSS that is publicly released, has an active community, and can demonstrate adoption. There is a rather large governance organization that would frown on the use of forked OSS libraries that were not getting the same vetting process, whatever level that vetting process might be by project. Additionally, maintaining release compatibility would likely run afoul of the same governance oversight (even with the simple text-book use of Vendor branches in CVS, SVN, etc). However, when Stripes can be released through Maven, this would make locally forked JAR releases easier to manage through a local Maven repository.

Without providing an incredible amount of detail, just know that these policies exist and can come bundled with incredible amounts of documentation in large organizations.

If anyone should download and review the patch, you'll note that I'm using PageContext.PAGE_SCOPE in the patch. I did use PageContext.REQUEST_SCOPE per Nikolaos recommendation, but did not want to upload another patch with just this change, until I got it working.

Show
Timothy Stone added a comment - 03/Jun/10 11:46 AM No need to apologize. Thanks for replying. As a Stripes advocate, the technical lead of a team of HTML developers, and the internal standards creator for HTML, JS and CSS coding, I'm not without my passion regarding this improvement. > I still don't get your explanation about the local modification part. > Using OSS is OK, but using customized OSS is treated like a security risks or something? > Sorry for my ignorance, but it doesn't make sense to me. My rather large, international organization endorses the use of OSS that is publicly released, has an active community, and can demonstrate adoption. There is a rather large governance organization that would frown on the use of forked OSS libraries that were not getting the same vetting process, whatever level that vetting process might be by project. Additionally, maintaining release compatibility would likely run afoul of the same governance oversight (even with the simple text-book use of Vendor branches in CVS, SVN, etc). However, when Stripes can be released through Maven, this would make locally forked JAR releases easier to manage through a local Maven repository. Without providing an incredible amount of detail, just know that these policies exist and can come bundled with incredible amounts of documentation in large organizations. If anyone should download and review the patch, you'll note that I'm using PageContext.PAGE_SCOPE in the patch. I did use PageContext.REQUEST_SCOPE per Nikolaos recommendation, but did not want to upload another patch with just this change, until I got it working.
Hide
Nikolaos added a comment - 03/Jun/10 12:05 PM

Timothy,

I agree with all the standards stuff you mention... however this comment:

>> However, when Stripes can be released through Maven, this would make locally forked JAR releases easier to manage through a local Maven repository.

Is odd. We have a local M2 repo today for 1.5.3 and 1.5.4-SNAPSHOT. This is not an issue and is quite simple to setup. Ask me offline if you like.

However, I disagree that a local M2 repo will make locally forked code releases easier to manage. You still will need to manage the patch and apply the patch to each and every release and the code will be forked which is a major maintenance problem in itself that a local M2 repo will not solve. Sure - it would help with the deployment of such forked code - but that is a deployment issue that has nothing to do with the argument for / against forking code. Just my 2 cents....

Show
Nikolaos added a comment - 03/Jun/10 12:05 PM Timothy, I agree with all the standards stuff you mention... however this comment: >> However, when Stripes can be released through Maven, this would make locally forked JAR releases easier to manage through a local Maven repository. Is odd. We have a local M2 repo today for 1.5.3 and 1.5.4-SNAPSHOT. This is not an issue and is quite simple to setup. Ask me offline if you like. However, I disagree that a local M2 repo will make locally forked code releases easier to manage. You still will need to manage the patch and apply the patch to each and every release and the code will be forked which is a major maintenance problem in itself that a local M2 repo will not solve. Sure - it would help with the deployment of such forked code - but that is a deployment issue that has nothing to do with the argument for / against forking code. Just my 2 cents....
Hide
Iwao AVE! added a comment - 04/Jun/10 4:15 AM

Thanks for the explanation.
Aside from the technical aspect, I understand the difficulty of forking OSS in some projects.


> Correction... it is NOT the "occasional" warning... the warning occurs 100% of the time with HTML 4.01 validation.

I used the word 'occasional' because I thought it has a taste of 'temporal' (another example of my poor English skill).
You would have seen many things (html dialects, doc types, css hacks, etc.) come and go if you're working long enough as a web developer.
That's why I feel uneasy when a patch trying to absorb specific html dialect or browsers' misbehavior is proposed against the framework core.
It is relatively easy to add a new option, but it's difficult (if possible) to remove it afterwards, you know.

What if we have a way to override the behavior of original stripes tags by a extension mechanism like the one Stripes already uses for other components.
This, of course, must be a bigger change than the one you are working on, but would be a better/cleaner solution in a long run.
How do you think?

  1. I believe this was proposed by someone a while ago, but can't remember when and where. STS-47 maybe?


Lastly, let me interpret my comment about Struts...
Based on the KISS principle, what I was trying to say is: 'any sensible developers would choose Stripes for its simplicity (like we did)'.
I thought it was obvious enough, sorry.

Show
Iwao AVE! added a comment - 04/Jun/10 4:15 AM Thanks for the explanation. Aside from the technical aspect, I understand the difficulty of forking OSS in some projects. – > Correction... it is NOT the "occasional" warning... the warning occurs 100% of the time with HTML 4.01 validation. I used the word 'occasional' because I thought it has a taste of 'temporal' (another example of my poor English skill). You would have seen many things (html dialects, doc types, css hacks, etc.) come and go if you're working long enough as a web developer. That's why I feel uneasy when a patch trying to absorb specific html dialect or browsers' misbehavior is proposed against the framework core. It is relatively easy to add a new option, but it's difficult (if possible) to remove it afterwards, you know. What if we have a way to override the behavior of original stripes tags by a extension mechanism like the one Stripes already uses for other components. This, of course, must be a bigger change than the one you are working on, but would be a better/cleaner solution in a long run. How do you think?
  1. I believe this was proposed by someone a while ago, but can't remember when and where. STS-47 maybe?
– Lastly, let me interpret my comment about Struts... Based on the KISS principle, what I was trying to say is: 'any sensible developers would choose Stripes for its simplicity (like we did)'. I thought it was obvious enough, sorry.
Hide
Nikolaos added a comment - 04/Jun/10 11:05 AM

Iwao,

> You would have seen many things (html dialects, doc types, css hacks, etc.) come and go if you're working long enough as a web developer.

Ummm... sure... technology is "always" evolving... and that is why software is continuously upgraded... NO? Or should Stripes have say never leveraged Java annotations for example because... you know... one day they will be superseded by something else. Your really missing the point... I suggest understanding what is being proposed by EXAMINING the proposed code VS. arguing in general terms might be worth your time...

> That's why I feel uneasy when a patch trying to absorb specific html dialect or browsers' misbehavior is proposed against the framework core.

What browser misbehaviour? Come on let's not start adding FUD to get our point across. As for the other half of your point:

HTML 4.01 and XHTML 1.0/1.1 are current technologies. HTML 5 is an emerging new standard. You either develop HTML web pages OR you develop XHTML web pages... unless there is some other secret sauce I don't understand... and you have been around long enough and you develop web pages in some other technology.

Moreover, this may be a SHOCKER to you but Stripes Tags PRODUCE XHTML code... yes INDEED a Tag library by its very nature will produce an element that guess what... is XHTML or HTML compliant code.

Furthermore, Stripes today assumes that EVERYONE is developing XHTML compliant code which is INCORRECT. I develop HTML 4.01 web pages today and will most likely develop HTML 5.0 a year from now both of which WHEN validated NOT by the web browser but by a w3c validator - AND if you have been in the game as long as you say you have then I assume you do indeed VALIDATE your code (Right?) - will result in validation warnings TODAY and TOMORROW.

Of course one can ignore validation warnings so in your opinion Stripes Tags should NOT produce CORRECT code 100% of the time. Interesting logic and interesting point indeed. Why bother... right... its going to change 3 years from now... .

> It is relatively easy to add a new option, but it's difficult (if possible) to remove it afterwards, you know.

WHY in the world would you want to remove it?????? Unless you envision a world were in HTML 5 will not get released and HTML 4.01 doesn't exist and 100% of all web pages produced from today onward will be XHTML compliant WHY IN THE WORLD would you want to remove this improvement. It's an improvement for a reason... .

ALSO guess what... developers do ZERO once the improvement is in place and Stripes Tags will continue to assume XHTML compliant code. Since you haven't read the code and I doubt you ever will but will argue regardless here is the nutshell...

TODAY:

Stripes Tags produces code that is XHTML compliant

After improvement:

a) xhtml=true OR no explicit declaration and Stripes Tags has an extra if condition that produces XHTML compliant code
a) xhtml=false and Stripes Tags has an extra if condition that produces HTML compliant code

How is that a bad thing?????

> What if we have a way to override the behavior of original stripes tags by a extension mechanism like the one Stripes already uses for other components. This, of course, must be a bigger change than the one you are working on, but would be a better/cleaner solution in a long run. How do you think?

Seriously... how would that EVEN work? Here you argue against a simple variable and if condition in a Tag library and suggest baking an extension that would somehow hook into the framework that the Stripes Tag libraries would have to look for and somehow leverage. Wow... . So what do I think... an awful idea... that many would be very much against.

The improvement relates to Stripes Tags NOT the Stripes framework code. If you understood that separation and where the change is proposed I imagine you wouldn't be making this suggestion.

I think you need to take 5 minutes to look at the proposed code.

> Lastly, let me interpret my comment about Struts...Based on the KISS principle, what I was trying to say is: 'any sensible developers would choose Stripes for its simplicity (like we did)'. I thought it was obvious enough, sorry.

Yes and any sensible developer that chooses a framework simple or not would like to see it evolve and improve. If you believe that Stripes should be baselined today and anyone who wants to improve it should look at Struts even if they suggest adding a warranted improvement with ZERO additional configuration for the developer then I would say I understand your argument

Here's some irony... this improvement uses the KISS principle... backwards compatible... ZERO additional configuration... encapsulated smarter Tag libraries... yet you argue against it... . I'm at a loss... and am done.

Show
Nikolaos added a comment - 04/Jun/10 11:05 AM Iwao, > You would have seen many things (html dialects, doc types, css hacks, etc.) come and go if you're working long enough as a web developer. Ummm... sure... technology is "always" evolving... and that is why software is continuously upgraded... NO? Or should Stripes have say never leveraged Java annotations for example because... you know... one day they will be superseded by something else. Your really missing the point... I suggest understanding what is being proposed by EXAMINING the proposed code VS. arguing in general terms might be worth your time... > That's why I feel uneasy when a patch trying to absorb specific html dialect or browsers' misbehavior is proposed against the framework core. What browser misbehaviour? Come on let's not start adding FUD to get our point across. As for the other half of your point: HTML 4.01 and XHTML 1.0/1.1 are current technologies. HTML 5 is an emerging new standard. You either develop HTML web pages OR you develop XHTML web pages... unless there is some other secret sauce I don't understand... and you have been around long enough and you develop web pages in some other technology. Moreover, this may be a SHOCKER to you but Stripes Tags PRODUCE XHTML code... yes INDEED a Tag library by its very nature will produce an element that guess what... is XHTML or HTML compliant code. Furthermore, Stripes today assumes that EVERYONE is developing XHTML compliant code which is INCORRECT. I develop HTML 4.01 web pages today and will most likely develop HTML 5.0 a year from now both of which WHEN validated NOT by the web browser but by a w3c validator - AND if you have been in the game as long as you say you have then I assume you do indeed VALIDATE your code (Right?) - will result in validation warnings TODAY and TOMORROW. Of course one can ignore validation warnings so in your opinion Stripes Tags should NOT produce CORRECT code 100% of the time. Interesting logic and interesting point indeed. Why bother... right... its going to change 3 years from now... . > It is relatively easy to add a new option, but it's difficult (if possible) to remove it afterwards, you know. WHY in the world would you want to remove it?????? Unless you envision a world were in HTML 5 will not get released and HTML 4.01 doesn't exist and 100% of all web pages produced from today onward will be XHTML compliant WHY IN THE WORLD would you want to remove this improvement. It's an improvement for a reason... . ALSO guess what... developers do ZERO once the improvement is in place and Stripes Tags will continue to assume XHTML compliant code. Since you haven't read the code and I doubt you ever will but will argue regardless here is the nutshell... TODAY: Stripes Tags produces code that is XHTML compliant After improvement: a) xhtml=true OR no explicit declaration and Stripes Tags has an extra if condition that produces XHTML compliant code a) xhtml=false and Stripes Tags has an extra if condition that produces HTML compliant code How is that a bad thing????? > What if we have a way to override the behavior of original stripes tags by a extension mechanism like the one Stripes already uses for other components. This, of course, must be a bigger change than the one you are working on, but would be a better/cleaner solution in a long run. How do you think? Seriously... how would that EVEN work? Here you argue against a simple variable and if condition in a Tag library and suggest baking an extension that would somehow hook into the framework that the Stripes Tag libraries would have to look for and somehow leverage. Wow... . So what do I think... an awful idea... that many would be very much against. The improvement relates to Stripes Tags NOT the Stripes framework code. If you understood that separation and where the change is proposed I imagine you wouldn't be making this suggestion. I think you need to take 5 minutes to look at the proposed code. > Lastly, let me interpret my comment about Struts...Based on the KISS principle, what I was trying to say is: 'any sensible developers would choose Stripes for its simplicity (like we did)'. I thought it was obvious enough, sorry. Yes and any sensible developer that chooses a framework simple or not would like to see it evolve and improve. If you believe that Stripes should be baselined today and anyone who wants to improve it should look at Struts even if they suggest adding a warranted improvement with ZERO additional configuration for the developer then I would say I understand your argument Here's some irony... this improvement uses the KISS principle... backwards compatible... ZERO additional configuration... encapsulated smarter Tag libraries... yet you argue against it... . I'm at a loss... and am done.
Hide
Levi Hoogenberg added a comment - 04/Jun/10 12:16 PM

Nikolaos, I feel that your tone is both uncalled for and distracting from your technical arguments. I for one would love Stripes-based discussions to remain as civil as they have always been, but others' views may vary.

Show
Levi Hoogenberg added a comment - 04/Jun/10 12:16 PM Nikolaos, I feel that your tone is both uncalled for and distracting from your technical arguments. I for one would love Stripes-based discussions to remain as civil as they have always been, but others' views may vary.
Hide
Nikolaos added a comment - 04/Jun/10 12:56 PM

Levi, you know what... your right. I apologize as my tone indeed is one of frustration... and I agree I should have stuck to the technical comments only.

I take full responsibility for allowing myself to be triggered by all the seemingly ill-researched / backed comments with the last trigger being:
>> # And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so?

Everyone is entitled to their comments... and any intended sarcasm aside... I sincerely worry if this is a sentiment shared by the Stripes community at large.

My sincere apologies Iwao and to anyone who may have been offended.

Show
Nikolaos added a comment - 04/Jun/10 12:56 PM Levi, you know what... your right. I apologize as my tone indeed is one of frustration... and I agree I should have stuck to the technical comments only. I take full responsibility for allowing myself to be triggered by all the seemingly ill-researched / backed comments with the last trigger being: >> # And if somebody chose Struts because Stripes has less options, why don't we just let him/her do so? Everyone is entitled to their comments... and any intended sarcasm aside... I sincerely worry if this is a sentiment shared by the Stripes community at large. My sincere apologies Iwao and to anyone who may have been offended.
Hide
Ben Gunter added a comment - 04/Jun/10 1:11 PM

Wow. You guys must really hate HTML validator warnings.

Show
Ben Gunter added a comment - 04/Jun/10 1:11 PM Wow. You guys must really hate HTML validator warnings.
Hide
Ben Gunter added a comment - 04/Jun/10 1:20 PM

On a serious note, I do have some major misgivings about implementing this as an attribute to a layout tag. Layouts are not required for use with other tags that happen to emit HTML so I don't think it's appropriate to require use of layout tags to enable or disable XHTML. We were thinking something more along the lines of a simple tag that could be inserted at the top of a page to set options ...

<s:options xhtml="false" />

or something like that.

Show
Ben Gunter added a comment - 04/Jun/10 1:20 PM On a serious note, I do have some major misgivings about implementing this as an attribute to a layout tag. Layouts are not required for use with other tags that happen to emit HTML so I don't think it's appropriate to require use of layout tags to enable or disable XHTML. We were thinking something more along the lines of a simple tag that could be inserted at the top of a page to set options ... <s:options xhtml="false" /> or something like that.
Hide
Timothy Stone added a comment - 04/Jun/10 1:41 PM

@Ben, I'll look at that tonight. Thanks for the idea.

Show
Timothy Stone added a comment - 04/Jun/10 1:41 PM @Ben, I'll look at that tonight. Thanks for the idea.
Hide
Nikolaos added a comment - 04/Jun/10 1:52 PM

> Wow. You guys must really hate HTML validator warnings.

LOL. Actually if you do regularly validate and use HTML doc type this is more annoying than it appears... It's 1 warning "per" tag that is not correct... .

So if you have a form with 5, 10 or whatever improperly closed elements on a page you could easily get a dozen or more extraneous warnings you need to wade through to with "each" of the following form:

  1. Warning Line 5, Column 51: NET-enabling start-tag requires SHORTTAG YES
    <p>First name:<input type="text" name="firstname" /></p>
    The sequence <FOO /> can be interpreted in at least two different ways, depending on the DOCTYPE of the document. For HTML 4.01 Strict, the '/' terminates the tag <FOO (with an implied '>'). However, since many browsers don't interpret it this way, even in the presence of an HTML 4.01 Strict DOCTYPE, it is best to avoid it completely in pure HTML documents and reserve its use solely for those written in XHTML.

Finding your legitimate warnings and errors in the enlarged list is the "real" annoying part

> On a serious note, I do have some major misgivings about implementing this as an attribute to a layout tag. Layouts are not required for use with other tags that happen to emit HTML so I don't think it's appropriate to require use of layout tags to enable or disable XHTML. We were thinking something more along the lines of a simple tag that could be inserted at the top of a page to set options ...
>
> <s:options xhtml="false" />
>
> or something like that.

Great point. Decoupling the mechanism from the Layout tag would allow more flexibility e.g. for HTML fragments w/ or w/o Ajax, etc...

Show
Nikolaos added a comment - 04/Jun/10 1:52 PM > Wow. You guys must really hate HTML validator warnings. LOL. Actually if you do regularly validate and use HTML doc type this is more annoying than it appears... It's 1 warning "per" tag that is not correct... . So if you have a form with 5, 10 or whatever improperly closed elements on a page you could easily get a dozen or more extraneous warnings you need to wade through to with "each" of the following form:
  1. Warning Line 5, Column 51: NET-enabling start-tag requires SHORTTAG YES <p>First name:<input type="text" name="firstname" /></p> The sequence <FOO /> can be interpreted in at least two different ways, depending on the DOCTYPE of the document. For HTML 4.01 Strict, the '/' terminates the tag <FOO (with an implied '>'). However, since many browsers don't interpret it this way, even in the presence of an HTML 4.01 Strict DOCTYPE, it is best to avoid it completely in pure HTML documents and reserve its use solely for those written in XHTML.
Finding your legitimate warnings and errors in the enlarged list is the "real" annoying part > On a serious note, I do have some major misgivings about implementing this as an attribute to a layout tag. Layouts are not required for use with other tags that happen to emit HTML so I don't think it's appropriate to require use of layout tags to enable or disable XHTML. We were thinking something more along the lines of a simple tag that could be inserted at the top of a page to set options ... > > <s:options xhtml="false" /> > > or something like that. Great point. Decoupling the mechanism from the Layout tag would allow more flexibility e.g. for HTML fragments w/ or w/o Ajax, etc...
Hide
Iwao AVE! added a comment - 07/Jun/10 10:09 AM

No apologies to me, Nikolaos.
I should have been more careful.

Show
Iwao AVE! added a comment - 07/Jun/10 10:09 AM No apologies to me, Nikolaos. I should have been more careful.
Hide
Iwao AVE! added a comment - 08/Sep/10 7:58 AM

It's been a while and Timothy might have finished his patch, but please let me add another possible solution.

The attached stripes-html4-input-0.1.jar is an extension that makes Stripes generate html4-compatible input tags.
Note that it is just a proof-of-concept and is incomplete at this point (_sourcePage and __fp hidden tags are self-closing) because the core FormTag is not designed to be subclassed.

To use the extension :
1. drop the jar into /WEB-INF/lib directory (Stripes 1.5.3 is required).
2. change the 'uri' attribute of taglib directive that declares stripes tags from
http://stripes.sourceforge.net/stripes.tld
to
http://stripes.sourceforge.net/stripes-html4-input.tld

Although it requires minor optimizations to the core tags to complete, it does not require neither a new option nor local forking.
Couldn't it be a better solution to the problem?

Show
Iwao AVE! added a comment - 08/Sep/10 7:58 AM It's been a while and Timothy might have finished his patch, but please let me add another possible solution. The attached stripes-html4-input-0.1.jar is an extension that makes Stripes generate html4-compatible input tags. Note that it is just a proof-of-concept and is incomplete at this point (_sourcePage and __fp hidden tags are self-closing) because the core FormTag is not designed to be subclassed. To use the extension : 1. drop the jar into /WEB-INF/lib directory (Stripes 1.5.3 is required). 2. change the 'uri' attribute of taglib directive that declares stripes tags from http://stripes.sourceforge.net/stripes.tld to http://stripes.sourceforge.net/stripes-html4-input.tld Although it requires minor optimizations to the core tags to complete, it does not require neither a new option nor local forking. Couldn't it be a better solution to the problem?
Iwao AVE! made changes - 08/Sep/10 7:58 AM
Attachment stripes-html4-input-0.1.jar [ 10323 ]
Hide
Timothy Stone added a comment - 08/Sep/10 9:34 AM

Indeed. I got stuck shortly after Ben's recommendation of a singular tag, e.g., <s:options ...>.

I revisited this just last week with the recent activity on the list and I think I have a solution that I need to actually put into a working application for smoke testing (possibly today)

I wanted something that was "set and forget" (as much as possible like Struts). Initially, I thought that the LayoutDefinition would be a good place, but nested definitions and the framework seemed to lose scope of the request where the attribute was set. It was always coming back null for the attribute.

Ben had misgivings about placing the attribute in the Layout tags, and I had misgivings about an <s:options ...> tag. it felt too much like the HTML select element tag extension, <s:option ...> (where one might see <s:options ...> as a way to have single tag in an <s:select ...>).

I have started looking at the <s:form ...> a bit more. The issue lies in the input elements generated by Stripes out-of-the-box in <s:form...> or <s:form partial='true' ...>, not with template text that might be in a Layout tag.

My current approach adds a new attribute to <s:form ...>. Input tags can get the parent form tag via getParentFormTag and then can call an overridden writeSingletonTag method, or set it's own PageScope'd variable that can branch the existing element terminator based on the form element.

I'll have something for review EOD or tomorrow.

Show
Timothy Stone added a comment - 08/Sep/10 9:34 AM Indeed. I got stuck shortly after Ben's recommendation of a singular tag, e.g., <s:options ...>. I revisited this just last week with the recent activity on the list and I think I have a solution that I need to actually put into a working application for smoke testing (possibly today) I wanted something that was "set and forget" (as much as possible like Struts). Initially, I thought that the LayoutDefinition would be a good place, but nested definitions and the framework seemed to lose scope of the request where the attribute was set. It was always coming back null for the attribute. Ben had misgivings about placing the attribute in the Layout tags, and I had misgivings about an <s:options ...> tag. it felt too much like the HTML select element tag extension, <s:option ...> (where one might see <s:options ...> as a way to have single tag in an <s:select ...>). I have started looking at the <s:form ...> a bit more. The issue lies in the input elements generated by Stripes out-of-the-box in <s:form...> or <s:form partial='true' ...>, not with template text that might be in a Layout tag. My current approach adds a new attribute to <s:form ...>. Input tags can get the parent form tag via getParentFormTag and then can call an overridden writeSingletonTag method, or set it's own PageScope'd variable that can branch the existing element terminator based on the form element. I'll have something for review EOD or tomorrow.
Hide
Nikolaos added a comment - 08/Sep/10 10:39 AM

Iwao,

Can you please explain what it is your POC does? i.e. how it attempts to solve this problem. Love to look at the code but an overview would be better for all.

Timothy,

1) When you were testing your patch code was it on 1.5.4? Just curious because the nested layout stuff is broken there and is a known issue.

2) Also as far as your 2nd option is concerned by embedding something in the <form> tag are we not ignoring other tags that might need to be output appropriately... i.e. it doesn't feel right as an entire document would be in HTML or XHTML not just the <form> tag.

3) I agree with Ben but if the issue is the name he picked for the tag (which I agree is way too similar to the HTML options) then why not simply come up with another name for the tag. I don't think Ben spent a whole lot of time thinking about a specific name but rather was stating that he didn't like the initial suggestion. So how about something like the following:

<s:doctypeOptions xhtml="false" />

All,

Before cranking out code let's put on the table what the best approach / solution might be to solve this issue... .

Then someone can go away and work out a little POC or something... .

Makes sense?

--Nikolaos

Show
Nikolaos added a comment - 08/Sep/10 10:39 AM Iwao, Can you please explain what it is your POC does? i.e. how it attempts to solve this problem. Love to look at the code but an overview would be better for all. Timothy, 1) When you were testing your patch code was it on 1.5.4? Just curious because the nested layout stuff is broken there and is a known issue. 2) Also as far as your 2nd option is concerned by embedding something in the <form> tag are we not ignoring other tags that might need to be output appropriately... i.e. it doesn't feel right as an entire document would be in HTML or XHTML not just the <form> tag. 3) I agree with Ben but if the issue is the name he picked for the tag (which I agree is way too similar to the HTML options) then why not simply come up with another name for the tag. I don't think Ben spent a whole lot of time thinking about a specific name but rather was stating that he didn't like the initial suggestion. So how about something like the following: <s:doctypeOptions xhtml="false" /> All, Before cranking out code let's put on the table what the best approach / solution might be to solve this issue... . Then someone can go away and work out a little POC or something... . Makes sense? --Nikolaos
Hide
Iwao AVE! added a comment - 08/Sep/10 11:46 AM

Nikolaos,

OK, let me try

The jar contains (a) subclasses of some Stripes input tags and (b) TLDs.

(a) override super class' methods to omit the self-closing slash when they are rendered.

For (b), I have copied the original Stripes' TLDs and replaced class references of input tags to those of (a).
Note that it still references original non-input Stripes tags.

Now, by changing the TLD reference (the step 2 in my previous comment), all the Stripes input tags in your JSPs are rendered by the new custom tags (a) which don't output the self-closing slash.

As you would have only one place to declare the TLD reference, it's quite easy to try it on an existing webapp (if it uses Stripes 1.5.3 or later).
Further questions or comments are welcome.
I too am looking for the best approach here.

  1. And the source files are included in the jar which I forgot to mention.

Regards,
Iwao

Show
Iwao AVE! added a comment - 08/Sep/10 11:46 AM Nikolaos, OK, let me try The jar contains (a) subclasses of some Stripes input tags and (b) TLDs. (a) override super class' methods to omit the self-closing slash when they are rendered. For (b), I have copied the original Stripes' TLDs and replaced class references of input tags to those of (a). Note that it still references original non-input Stripes tags. Now, by changing the TLD reference (the step 2 in my previous comment), all the Stripes input tags in your JSPs are rendered by the new custom tags (a) which don't output the self-closing slash. As you would have only one place to declare the TLD reference, it's quite easy to try it on an existing webapp (if it uses Stripes 1.5.3 or later). Further questions or comments are welcome. I too am looking for the best approach here.
  1. And the source files are included in the jar which I forgot to mention.
Regards, Iwao
Hide
Nikolaos added a comment - 08/Sep/10 1:45 PM

Okay Iwao. Thanks for the feedback.

So here are the options thus far on the table:

1) Timothy's suggestion: Adding an additional optional argument to LayoutDefinition ala Struts that defaults to xhtml=true for backwards compatability.

Cons: Not all output is HTML fragments and not all output occurs within a LayoutDefinition tag (e.g. someone may be using Tiles or SiteMesh); this is besides the issues of getting this work with nested Layouts (which are problematic in 1.5.4 Snapshot).

2) Iwao's suggestion: Pretty simple... swap out the implemented Stripes jar with one that hard codes for HTML vs XHTML

Cons: Too rigid in that in order to produce the correct HTML / XHTML markup I need to swap out my version of Stripes. Also how is this maintained down the road? What happens with version 1.5.4 or 1.6.0... will we always maintain 2 versions of the libraries. I know you were simply offering a solution which is great but I don't think any one developer exclusively works with HTML or XHTML documents and even if they did the maintenance and management of this would be too arduous. A solution definitely. But I would be apt to tolerate the current validation warnings than go down this avenue.

3) Timothy's 2nd suggestion: Add the optional argument to a <form> tag.

Cons: It appears that as LayoutDefinition didn't seem appropriate as per Con in 1) above and that this alternative suggestion was to be very specific and put it on the <form> tag but that doesn't work for other reasons. Primarily what if I have output that does not include a form tag... e.g. what if I just have Ajax output or some other fragment. We need a more flexible solution.

4) Ben's suggestion tweak on Timothy's suggestion which I simply renamed as: <s:doctypeOptions xhtml="false" />

Cons: A new tag needs to be implemented / authored and documented.

In fact, although developers may work with both HTML and XHTML one thing is certain... for any given web page the document or fragment is either HTML or XHTML (or of course JS, etc...) but never both HTML and XHTML interspersed. What that tells me is that the ideal solution should be configurable dynamically and work equally well on an entire web page or even a fragment and should be flexible enough to turn on or off for a given page. Solution 4) provides that IMO.

What do others think? Any other solutions?

--Nikolaos

Show
Nikolaos added a comment - 08/Sep/10 1:45 PM Okay Iwao. Thanks for the feedback. So here are the options thus far on the table: 1) Timothy's suggestion: Adding an additional optional argument to LayoutDefinition ala Struts that defaults to xhtml=true for backwards compatability. Cons: Not all output is HTML fragments and not all output occurs within a LayoutDefinition tag (e.g. someone may be using Tiles or SiteMesh); this is besides the issues of getting this work with nested Layouts (which are problematic in 1.5.4 Snapshot). 2) Iwao's suggestion: Pretty simple... swap out the implemented Stripes jar with one that hard codes for HTML vs XHTML Cons: Too rigid in that in order to produce the correct HTML / XHTML markup I need to swap out my version of Stripes. Also how is this maintained down the road? What happens with version 1.5.4 or 1.6.0... will we always maintain 2 versions of the libraries. I know you were simply offering a solution which is great but I don't think any one developer exclusively works with HTML or XHTML documents and even if they did the maintenance and management of this would be too arduous. A solution definitely. But I would be apt to tolerate the current validation warnings than go down this avenue. 3) Timothy's 2nd suggestion: Add the optional argument to a <form> tag. Cons: It appears that as LayoutDefinition didn't seem appropriate as per Con in 1) above and that this alternative suggestion was to be very specific and put it on the <form> tag but that doesn't work for other reasons. Primarily what if I have output that does not include a form tag... e.g. what if I just have Ajax output or some other fragment. We need a more flexible solution. 4) Ben's suggestion tweak on Timothy's suggestion which I simply renamed as: <s:doctypeOptions xhtml="false" /> Cons: A new tag needs to be implemented / authored and documented. In fact, although developers may work with both HTML and XHTML one thing is certain... for any given web page the document or fragment is either HTML or XHTML (or of course JS, etc...) but never both HTML and XHTML interspersed. What that tells me is that the ideal solution should be configurable dynamically and work equally well on an entire web page or even a fragment and should be flexible enough to turn on or off for a given page. Solution 4) provides that IMO. What do others think? Any other solutions? --Nikolaos
Hide
Timothy Stone added a comment - 09/Nov/10 2:16 PM

I now have a complete patch... maybe just in time for the closing of v1.5.4.

See attached "stripes-html-config.tgz" for diffs.

After some sputtering and rehashing, I settled on the idea that the output desired should be a deploy time configuration.

I wrestled with making it an Application configuration (context-param) or a Stripes Filter configuration (init-param), and settled in making the solution as "Stripey" as possible with a Stripes Filter setting, following the RuntimeConfiguration documentation.

The following shows the application configuration set by the deployer as an init-param to the Stripes Filter to "HTML" (the default remains XHTML and does not require configuration):

<filter>
<filter-name>
StripesFilter
</filter-name>
...
<init-param>
<param-name>
Stripes.HtmlMode
</param-name>
<param-value>
HTML
</param-value>
</init-param>
</filter>

This configuration is called internally in the FormTag, HtmlTagSupport and InputOptionsCollectionTag (or every where Stripes closed empty elements with " />"), e.g., "html".equalsIgnoreCase(StripesFilter.getConfiguration().getHtmlMode())

Backward compatibility is maintained and the default is "as it exists today." The deployer, on advice from Front End Engineers, can set the Stripes Filter to use the HTML 4 compatible tag syntax (also compatible with HTML 5).

Hope this meets everyone's expectations of default nature and configuration while remaining "Stripey."

Show
Timothy Stone added a comment - 09/Nov/10 2:16 PM I now have a complete patch... maybe just in time for the closing of v1.5.4. See attached "stripes-html-config.tgz" for diffs. After some sputtering and rehashing, I settled on the idea that the output desired should be a deploy time configuration. I wrestled with making it an Application configuration (context-param) or a Stripes Filter configuration (init-param), and settled in making the solution as "Stripey" as possible with a Stripes Filter setting, following the RuntimeConfiguration documentation. The following shows the application configuration set by the deployer as an init-param to the Stripes Filter to "HTML" (the default remains XHTML and does not require configuration): <filter> <filter-name> StripesFilter </filter-name> ... <init-param> <param-name> Stripes.HtmlMode </param-name> <param-value> HTML </param-value> </init-param> </filter> This configuration is called internally in the FormTag, HtmlTagSupport and InputOptionsCollectionTag (or every where Stripes closed empty elements with " />"), e.g., "html".equalsIgnoreCase(StripesFilter.getConfiguration().getHtmlMode()) Backward compatibility is maintained and the default is "as it exists today." The deployer, on advice from Front End Engineers, can set the Stripes Filter to use the HTML 4 compatible tag syntax (also compatible with HTML 5). Hope this meets everyone's expectations of default nature and configuration while remaining "Stripey."
Hide
Timothy Stone added a comment - 09/Nov/10 2:18 PM

Supports HTML 4 (and HTML 5 compatible) self-closing, or empty, tag syntax through the configuration of the Stripes Filter at deployment time and runtime.

<filter>
<filter-name>
StripesFilter
</filter-name>
<filter-class>
net.sourceforge.stripes.controller.StripesFilter
</filter-class>
... other init-params...
<init-param>
<param-name>
Stripes.HtmlMode
</param-name>
<param-value>
HTML
</param-value>
</init-param>
</filter>

Show
Timothy Stone added a comment - 09/Nov/10 2:18 PM Supports HTML 4 (and HTML 5 compatible) self-closing, or empty, tag syntax through the configuration of the Stripes Filter at deployment time and runtime. <filter> <filter-name> StripesFilter </filter-name> <filter-class> net.sourceforge.stripes.controller.StripesFilter </filter-class> ... other init-params... <init-param> <param-name> Stripes.HtmlMode </param-name> <param-value> HTML </param-value> </init-param> </filter>
Timothy Stone made changes - 09/Nov/10 2:18 PM
Attachment stripes-html-config.tgz [ 10340 ]
Ben Gunter made changes - 10/Nov/10 8:04 AM
Fix Version/s Release 1.5.5 [ 10130 ]
Hide
Timothy Stone added a comment - 11/Nov/10 11:53 AM

This single patch file, stripes_html_mode-1.5.5_1.patch, contains a summary view of the proposed changes,

The proposal provides an application level configuration that deployers can use to globally set the HTML output compatible for HTML4 and HTML5. The Stripes default remains to produce XHTML-compatible syntax for empty elements, e.g., <input type="text" ... />.

The new HtmlOptionsTag is provided to allow view developers an "escape hatch" from the RuntimeConfiguration setting. Thus allowing some views specific to client type produce HTML4/5 or XHTML syntax and visa versa.

Show
Timothy Stone added a comment - 11/Nov/10 11:53 AM This single patch file, stripes_html_mode-1.5.5_1.patch, contains a summary view of the proposed changes, The proposal provides an application level configuration that deployers can use to globally set the HTML output compatible for HTML4 and HTML5. The Stripes default remains to produce XHTML-compatible syntax for empty elements, e.g., <input type="text" ... />. The new HtmlOptionsTag is provided to allow view developers an "escape hatch" from the RuntimeConfiguration setting. Thus allowing some views specific to client type produce HTML4/5 or XHTML syntax and visa versa.
Timothy Stone made changes - 11/Nov/10 11:53 AM
Attachment stripes_html_mode-1.5.5_1.patch [ 10341 ]
Hide
Iwao AVE! added a comment - 12/Nov/10 8:52 AM

Timothy,
I had an impression that you treat HTML 4 and 5 in the same manner, but the self-closing void elements are permitted in HTML 5, right?
http://dev.w3.org/html5/spec/syntax.html#elements-0 (section 8.1.2.1)
http://wiki.whatwg.org/wiki/FAQ#Should_I_close_empty_elements_with_.2F.3E_or_.3E.3F

Nikolaos,
Thanks for summarizing. Let me clarify a few things.
You don't need to 'swap out' Stripes jar. The steps I wrote are exactly what you have to do.
The jar contains only several subclasses and each of them overrides only particular methods (i.e. it modifies the particular part of the original behavior at runtime).
So, you don't have to maintain it unless that particular part of the core tags are changed (stripes-html4-input-0.1.jar should work with 1.5.4 as it is).

Anyway, I suggest you to take a look at the source and give it a try.
It is not that bad as a workaround until HTML 5 becomes mainstream, IMHO.

Show
Iwao AVE! added a comment - 12/Nov/10 8:52 AM Timothy, I had an impression that you treat HTML 4 and 5 in the same manner, but the self-closing void elements are permitted in HTML 5, right? http://dev.w3.org/html5/spec/syntax.html#elements-0 (section 8.1.2.1) http://wiki.whatwg.org/wiki/FAQ#Should_I_close_empty_elements_with_.2F.3E_or_.3E.3F Nikolaos, Thanks for summarizing. Let me clarify a few things. You don't need to 'swap out' Stripes jar. The steps I wrote are exactly what you have to do. The jar contains only several subclasses and each of them overrides only particular methods (i.e. it modifies the particular part of the original behavior at runtime). So, you don't have to maintain it unless that particular part of the core tags are changed (stripes-html4-input-0.1.jar should work with 1.5.4 as it is). Anyway, I suggest you to take a look at the source and give it a try. It is not that bad as a workaround until HTML 5 becomes mainstream, IMHO.
Hide
Timothy Stone added a comment - 12/Nov/10 9:57 AM

Iwao,

Correct. Permitted in HTML 5. This is a nod to all that XHTML 1x work I and others advocated. The permitted use of the XHTML1 syntax is not a "recommendation" from WHATWG as I have been told by members of the group.

Specifically, Molly Holzschlag at Philly ETE 2010 made it pretty clear: the fact is HTML 5 permits the worst sort of HTML 3/4 code practices possible, e.g., implictly closed P, LI and other tags. This "permits" is never to be construed as "recommended." Otherwise, as developers we have not advanced, but regressed. Molly also went on to say that HTML 5 introduces new concepts for web programmers traditionally thinking only about pretty pages... "You're a Web APPLICATION programmer now people. Never had to think about scopes before? If you're going to be using HTML5 get ready for terms like Local, Session, Request, and Application Data Storage. It's not all CSS and copy-and-paste Javascript anymore."

Paraphrased a bit. But yes, HTML 4 and HTML 5 have a very common heritage.

Show
Timothy Stone added a comment - 12/Nov/10 9:57 AM Iwao, Correct. Permitted in HTML 5. This is a nod to all that XHTML 1x work I and others advocated. The permitted use of the XHTML1 syntax is not a "recommendation" from WHATWG as I have been told by members of the group. Specifically, Molly Holzschlag at Philly ETE 2010 made it pretty clear: the fact is HTML 5 permits the worst sort of HTML 3/4 code practices possible, e.g., implictly closed P, LI and other tags. This "permits" is never to be construed as "recommended." Otherwise, as developers we have not advanced, but regressed. Molly also went on to say that HTML 5 introduces new concepts for web programmers traditionally thinking only about pretty pages... "You're a Web APPLICATION programmer now people. Never had to think about scopes before? If you're going to be using HTML5 get ready for terms like Local, Session, Request, and Application Data Storage. It's not all CSS and copy-and-paste Javascript anymore." Paraphrased a bit. But yes, HTML 4 and HTML 5 have a very common heritage.
Hide
Iwao AVE! added a comment - 17/Nov/10 2:10 AM

Timothy,

Well, you seem to have no doubt about the importance of the option even if the syntax is valid in HTML 5 spec.
Then what else can I say ?

Anyway, I really appreciate your patience and sincere comments.

Regards,
Iwao

Show
Iwao AVE! added a comment - 17/Nov/10 2:10 AM Timothy, Well, you seem to have no doubt about the importance of the option even if the syntax is valid in HTML 5 spec. Then what else can I say ? Anyway, I really appreciate your patience and sincere comments. Regards, Iwao
Hide
Nikolaos added a comment - 17/Nov/10 10:58 AM

Iwao and Timothy,

I think we all agree that HTML 4 generates validation warnings and although it's not the end of the world it is not ideal.
Moreover though... Iwao is correct in that there are no warnings for explicitly closing tags using the current HTML5 validator (marked experimental mind you).

So what does this mean IMHO... well for one a mess... in permitting in HTML 5 both implicit and explicit closure of tags it boils down to developer preference to what they want to use. Which from my perspective means that Stripes "should" allow developers to choose between one over the other regardless of "recommended" or not b/c both will be valid.

It also means this is no longer an HTML vs. XHTML thing and as such we should not label it as such i.e. Timothy you would need to re-label your feature as a flag of "HTML" won't cut it when HTML5 becomes mainstream.

So I still think Timothy's patch is valid for different reasons thanks to Iwao's comments: because it boils down to developer preference for at least the future w.r.t. HTML 5 albeit the feature needs to be labeled something other than "HTML" and ideally something quite generic like an on/off switch of sorts... .

--Nikolaos

Show
Nikolaos added a comment - 17/Nov/10 10:58 AM Iwao and Timothy, I think we all agree that HTML 4 generates validation warnings and although it's not the end of the world it is not ideal. Moreover though... Iwao is correct in that there are no warnings for explicitly closing tags using the current HTML5 validator (marked experimental mind you). So what does this mean IMHO... well for one a mess... in permitting in HTML 5 both implicit and explicit closure of tags it boils down to developer preference to what they want to use. Which from my perspective means that Stripes "should" allow developers to choose between one over the other regardless of "recommended" or not b/c both will be valid. It also means this is no longer an HTML vs. XHTML thing and as such we should not label it as such i.e. Timothy you would need to re-label your feature as a flag of "HTML" won't cut it when HTML5 becomes mainstream. So I still think Timothy's patch is valid for different reasons thanks to Iwao's comments: because it boils down to developer preference for at least the future w.r.t. HTML 5 albeit the feature needs to be labeled something other than "HTML" and ideally something quite generic like an on/off switch of sorts... . --Nikolaos
Hide
Timothy Stone added a comment - 18/Nov/10 7:54 PM

This patch accounts for the recent refactoring of the FormTag for STS-771.

Show
Timothy Stone added a comment - 18/Nov/10 7:54 PM This patch accounts for the recent refactoring of the FormTag for STS-771.
Timothy Stone made changes - 18/Nov/10 7:54 PM
Attachment stripes_html_mode-1.5.5_2.patch [ 10350 ]
Hide
Ben Gunter added a comment - 23/Nov/10 10:06 AM

Timothy, I've attached a new patch that's basically just a trimmed-down version of your latest one. I renamed HtmlOptionsTag to PageOptionsTag so that we can implement non-HTML-specific options in the future using the same tag. I removed all the changes to the configuration interfaces and classes; PageOptionsTag just gets the property directly instead of relying on a method in Configuration. And I added a method to HtmlTagSupport that makes much of the code in your patch more concise.

If there are no objections, I'll commit this patch today.

Show
Ben Gunter added a comment - 23/Nov/10 10:06 AM Timothy, I've attached a new patch that's basically just a trimmed-down version of your latest one. I renamed HtmlOptionsTag to PageOptionsTag so that we can implement non-HTML-specific options in the future using the same tag. I removed all the changes to the configuration interfaces and classes; PageOptionsTag just gets the property directly instead of relying on a method in Configuration. And I added a method to HtmlTagSupport that makes much of the code in your patch more concise. If there are no objections, I'll commit this patch today.
Ben Gunter made changes - 23/Nov/10 10:06 AM
Attachment stripes_html_mode-1.5.5_3.patch [ 10351 ]
Hide
Iwao AVE! added a comment - 23/Nov/10 11:58 AM

Thank you for your work, Ben.
I have no more objection.

And thanks for the comment, Nikolaos.
Now I can be sure that I have made myself clear

Show
Iwao AVE! added a comment - 23/Nov/10 11:58 AM Thank you for your work, Ben. I have no more objection. And thanks for the comment, Nikolaos. Now I can be sure that I have made myself clear
Hide
Ben Gunter added a comment - 23/Nov/10 12:20 PM

You're welcome, Iwao, but Timothy did most of the heavy lifting.

Show
Ben Gunter added a comment - 23/Nov/10 12:20 PM You're welcome, Iwao, but Timothy did most of the heavy lifting.
Hide
Ben Gunter added a comment - 23/Nov/10 4:38 PM

Fixed for 1.5.5 and 1.6

Show
Ben Gunter added a comment - 23/Nov/10 4:38 PM Fixed for 1.5.5 and 1.6
Ben Gunter made changes - 23/Nov/10 4:38 PM
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Hide
Timothy Stone added a comment - 24/Nov/10 1:16 PM

Patch looks great. Thanks for the review!

I see now how you were able to still set the filter's configuration without changing the interface and classes. This was not originally clear to me in attacking the global setting, so I followed the examples already present.

"Verba Volant, Scripta Manent"
Words Fly, Code Remains
^ borrowed from Nicola Ken Barozzi (former Cocoon committer)

Show
Timothy Stone added a comment - 24/Nov/10 1:16 PM Patch looks great. Thanks for the review! I see now how you were able to still set the filter's configuration without changing the interface and classes. This was not originally clear to me in attacking the global setting, so I followed the examples already present. "Verba Volant, Scripta Manent" Words Fly, Code Remains ^ borrowed from Nicola Ken Barozzi (former Cocoon committer)
Hide
Ben Gunter added a comment - 22/May/12 7:27 AM

Closing resolved issues that haven't been updated in the last year.

Show
Ben Gunter added a comment - 22/May/12 7:27 AM Closing resolved issues that haven't been updated in the last year.
Ben Gunter made changes - 22/May/12 7:27 AM
Status Resolved [ 5 ] Closed [ 6 ]

People

  • Assignee:
    Unassigned
    Reporter:
    Nikolaos
Vote (2)
Watch (1)

Dates

  • Created:
    28/May/10 2:35 PM
    Updated:
    22/May/12 7:27 AM
    Resolved:
    23/Nov/10 4:38 PM