You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Bruno Aranda <br...@gmail.com> on 2006/01/22 21:27:21 UTC
Moving AddResource stuff to common
As you might have realised, I am moving the AddResource stuff to
commons. I am doing this because it will help to solve some old bugs
we have (such as MYFACES-152). I am going to explain this in detail
when I stabilise again the codebase after this moves.
Thanks for your patience,
Bruno
Re: Moving AddResource stuff to common
Posted by Sean Schofield <se...@gmail.com>.
> @document-tag: Well, the thing would be that all this functionality
> could be moved out to tomahawk - with a custom document tag, we would
> be able to do that. We would need to enforce that the user actually
> uses such a t:document tag then, and I'm not comfortable with doint
> that right now.
-1 for forcing the user to use <t:document>. An interesting solution
but ultimately I'm against it.
> Martin
Sean
Re: Moving AddResource stuff to common
Posted by Martin Marinschek <ma...@gmail.com>.
In the long run, I want to get rid of the dummy form in any case and
instead dynamically add a form component to the tree on the first view
run if that is possible.
In the short run, the current solution is the only one Bruno and I
have come up with - I do believe its better than the current state, I
do admit it has some disadvantages, though.
@AddResource: I'd certainly say we should get out all the extended
stuff to something in tomahawk again, if we can.
@document-tag: Well, the thing would be that all this functionality
could be moved out to tomahawk - with a custom document tag, we would
be able to do that. We would need to enforce that the user actually
uses such a t:document tag then, and I'm not comfortable with doint
that right now.
regards,
Martin
On 1/23/06, Simon Kitching <sk...@apache.org> wrote:
> Hi Bruno,
>
> Thanks for the info.
>
> On Sun, 2006-01-22 at 22:40 +0100, Bruno Aranda wrote:
> > With the current code, we cannot move f:view from where it is now, for
> > instance now we cannot include the head inside the jsf markup. This
> > could be solved moving the f:viewTag before the head, but this doesn't
> > not work in myfaces because in our ResponseWriterImpl we write stuff
> > (javascript for dummyForm, autoscroll...) when the viewTag is closed
> > (ResponseWriter.endDocument()). We should move that code out of
> > ResponseWriterImpl.endDocument() and put it in another place to be
> > renderer just before the closing </body> tag.
>
> Ok, so the problem is this doesn't work:
>
> <f:view>
> <head>
> </head>
> <body>
> </body>
> </f:view>
>
> because the f:view tag (ViewTag.class) has a doEndTag method that calls
> responseWriter.endDocument(). That is actually an instance of
> HtmlResponseWriterImpl, and that method generates output that is only
> valid before a </body> tag. The output it generates includes:
> * a "dummy form" for use by command components that are not within any
> explicit <h:form> tag.
> * javascript to implement the "auto_scroll" functionality
>
> Note that the responseWriter.endDocument call doesn't mean that no more
> output can be generated, just that no more JSF output can be generated.
> Literal text and non-JSF JSP tags can still occur in the page.
>
> Note also that doAfterBody does generate a lot of "output", by searching
> through the page generated so far and replacing "marker" strings with
> the necessary output. This isn't a problem as this works no matter
> whether </body> has been output or not; the marker strings indicate
> where the changes are needed.
>
>
> Despite being the author of (the reimplementation of) the
> ReducedHTMLParser class, I'm not particularly keen on that approach.
> It's not efficient nor particularly robust. The "marker string" approach
> is much simpler and therefore more reliable in general.
>
> So rather than move AddResource and ReducedHTMLParser into common to
> solve this, I'd be much keener to see a solution that doesn't involve
> them if one can be found.
>
> One approach to outputting stuff that should exist once only in a page
> is to output the data when first needed, and set a request attribute as
> a flag to ensure it isn't output multiple times. The dummy form cannot
> be output this way, because the first occurrence may be within a <form>
> tag, and a form cannot be within a form in html. However a couple of
> ideas pop to mind:
> (1)
> If first use is not within an h:form, output immediately. If first use
> is not within a form, then don't output but instead put a "need-dummy"
> flag in the request. The h:form tag checks at doEndTag whether a dummy
> form is needed, and if so outputs it. Issues: what if a user adds a
> literal <form> with JSF components in it rather than an h:form? [but
> will the current code do the right thing in this scenario anyway?]
> (2)
> On first use, generate a <script> tag that invokes a function to
> dynamically generate the dummy form. The "dummy form" stuff is only
> applicable when javascript is enabled, yes? Without javascript, command
> components cannot submit a form they are not physically within anyway.
> And javascript should be able to create a FORM node, populate it with
> the necessary INPUT nodes and attach it as a child to the <body> element
> at runtime.
>
>
> I should say that I don't really understand the role of the
> JspViewHandler, and in particular Jacob's emails saying that a lot of
> code currently in JSP tag handlers should be in the view handler
> instead. I'll look into that right now. Initially, it looks to me like
> he is objecting to the fact that h:form outputs special marker strings
> that the ViewTag class later locates and replaces with the saved state
> of the view tree. However in order to support client-side state saving
> for HTML, each form *must* contain that state info. So it looks like
> Jacob is pushing for the "post-parse the generated HTML page to find the
> form tags" approach, ie the ReducedHTMLParser approach, and to delete
> the "marker string" approach completely. Have I got that right? If so,
> what does the JspViewHandler have to do with this issue?
>
>
>
> > Alongside we will fix
> > two important bugs (MYFACES-152 [1] and MYFACES-883 [2]) that hinders
> > us to use adf-faces and facelets together with myfaces.
> >
> > We can use AddResource.java to write the stuff before the </body>. It
> > has been moved to commons because we should call to this class from a
> > base html renderer. Martin and I have been thinking that a good place
> > to call to AddResource could be the viewTag component, although there
> > are some issues against that as Jacob pointed out in a mail some time
> > ago [3].
>
> It would mean that ReducedHTMLParser would be parsing a document that
> may not yet be complete (ie may still be missing </body> and </html>
> tags. I can't see a problem there though.
>
> Currently, AddResource contains a lot of functionality that isn't
> relevant to JSF core, though. Things like adding body onload callbacks,
> or <link> tags into the head, or generating urls like
> "faces/myExtensionResource/*". In other words, lots of functionality
> will be included in the myfaces-impl.jar that is never used. Refactoring
> this class into core-only and extended functionality might be nicer.
>
> > Our proposal is to (1) to create a document tag (just like in
> > adf-faces) and call to AddResource from there and (2) also we could
> > also render it in the f:view tag if the document tag is not included
> > in the page. All this will solve those old bugs and we could have a
> > new document tag which may simplify things.
>
> What do you mean by "a document tag"? I presume you don't mean creating
> a <t:document>, as that can obviously not be used from core.
>
> >
> > One new issue after moving the AddResource.java file and its
> > dependencies to commons is that AddResourceTest uses some classes in
> > myfaces-impl. I left AddResourceTest in tomahawk while we reach a
> > solution for this.
> >
> > And of course, nothing has been decided, so thoughts and ideas welcome :-)
> >
> > Regards,
> >
> > Bruno
> >
> > [1] http://issues.apache.org/jira/browse/MYFACES-152
> > [2] http://issues.apache.org/jira/browse/MYFACES-883
> > [3] http://www.mail-archive.com/dev@myfaces.apache.org/msg07873.html
> >
> > 2006/1/22, Bruno Aranda <br...@gmail.com>:
> > > Yes, you are right sorry. I first thought that only two classes were
> > > involved, but after moving them my first priority was to stabilise
> > > again the code base. I am going to explain everything now...
> > >
> > > 2006/1/22, Simon Kitching <sk...@apache.org>:
> > > > On Sun, 2006-01-22 at 21:27 +0100, Bruno Aranda wrote:
> > > > > As you might have realised, I am moving the AddResource stuff to
> > > > > commons. I am doing this because it will help to solve some old bugs
> > > > > we have (such as MYFACES-152). I am going to explain this in detail
> > > > > when I stabilise again the codebase after this moves.
> > > > >
> > > > > Thanks for your patience,
> > > >
> > > > I was quite surprised to see the commit messages moving this class.
> > > > Perhaps some discussion first might have been a good idea?
> > > >
> > > > Regards,
> > > >
> > > > Simon
> > > >
> > > >
> > >
>
>
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: Moving AddResource stuff to common
Posted by Simon Kitching <sk...@apache.org>.
Hi Bruno,
Thanks for the info.
On Sun, 2006-01-22 at 22:40 +0100, Bruno Aranda wrote:
> With the current code, we cannot move f:view from where it is now, for
> instance now we cannot include the head inside the jsf markup. This
> could be solved moving the f:viewTag before the head, but this doesn't
> not work in myfaces because in our ResponseWriterImpl we write stuff
> (javascript for dummyForm, autoscroll...) when the viewTag is closed
> (ResponseWriter.endDocument()). We should move that code out of
> ResponseWriterImpl.endDocument() and put it in another place to be
> renderer just before the closing </body> tag.
Ok, so the problem is this doesn't work:
<f:view>
<head>
</head>
<body>
</body>
</f:view>
because the f:view tag (ViewTag.class) has a doEndTag method that calls
responseWriter.endDocument(). That is actually an instance of
HtmlResponseWriterImpl, and that method generates output that is only
valid before a </body> tag. The output it generates includes:
* a "dummy form" for use by command components that are not within any
explicit <h:form> tag.
* javascript to implement the "auto_scroll" functionality
Note that the responseWriter.endDocument call doesn't mean that no more
output can be generated, just that no more JSF output can be generated.
Literal text and non-JSF JSP tags can still occur in the page.
Note also that doAfterBody does generate a lot of "output", by searching
through the page generated so far and replacing "marker" strings with
the necessary output. This isn't a problem as this works no matter
whether </body> has been output or not; the marker strings indicate
where the changes are needed.
Despite being the author of (the reimplementation of) the
ReducedHTMLParser class, I'm not particularly keen on that approach.
It's not efficient nor particularly robust. The "marker string" approach
is much simpler and therefore more reliable in general.
So rather than move AddResource and ReducedHTMLParser into common to
solve this, I'd be much keener to see a solution that doesn't involve
them if one can be found.
One approach to outputting stuff that should exist once only in a page
is to output the data when first needed, and set a request attribute as
a flag to ensure it isn't output multiple times. The dummy form cannot
be output this way, because the first occurrence may be within a <form>
tag, and a form cannot be within a form in html. However a couple of
ideas pop to mind:
(1)
If first use is not within an h:form, output immediately. If first use
is not within a form, then don't output but instead put a "need-dummy"
flag in the request. The h:form tag checks at doEndTag whether a dummy
form is needed, and if so outputs it. Issues: what if a user adds a
literal <form> with JSF components in it rather than an h:form? [but
will the current code do the right thing in this scenario anyway?]
(2)
On first use, generate a <script> tag that invokes a function to
dynamically generate the dummy form. The "dummy form" stuff is only
applicable when javascript is enabled, yes? Without javascript, command
components cannot submit a form they are not physically within anyway.
And javascript should be able to create a FORM node, populate it with
the necessary INPUT nodes and attach it as a child to the <body> element
at runtime.
I should say that I don't really understand the role of the
JspViewHandler, and in particular Jacob's emails saying that a lot of
code currently in JSP tag handlers should be in the view handler
instead. I'll look into that right now. Initially, it looks to me like
he is objecting to the fact that h:form outputs special marker strings
that the ViewTag class later locates and replaces with the saved state
of the view tree. However in order to support client-side state saving
for HTML, each form *must* contain that state info. So it looks like
Jacob is pushing for the "post-parse the generated HTML page to find the
form tags" approach, ie the ReducedHTMLParser approach, and to delete
the "marker string" approach completely. Have I got that right? If so,
what does the JspViewHandler have to do with this issue?
> Alongside we will fix
> two important bugs (MYFACES-152 [1] and MYFACES-883 [2]) that hinders
> us to use adf-faces and facelets together with myfaces.
>
> We can use AddResource.java to write the stuff before the </body>. It
> has been moved to commons because we should call to this class from a
> base html renderer. Martin and I have been thinking that a good place
> to call to AddResource could be the viewTag component, although there
> are some issues against that as Jacob pointed out in a mail some time
> ago [3].
It would mean that ReducedHTMLParser would be parsing a document that
may not yet be complete (ie may still be missing </body> and </html>
tags. I can't see a problem there though.
Currently, AddResource contains a lot of functionality that isn't
relevant to JSF core, though. Things like adding body onload callbacks,
or <link> tags into the head, or generating urls like
"faces/myExtensionResource/*". In other words, lots of functionality
will be included in the myfaces-impl.jar that is never used. Refactoring
this class into core-only and extended functionality might be nicer.
> Our proposal is to (1) to create a document tag (just like in
> adf-faces) and call to AddResource from there and (2) also we could
> also render it in the f:view tag if the document tag is not included
> in the page. All this will solve those old bugs and we could have a
> new document tag which may simplify things.
What do you mean by "a document tag"? I presume you don't mean creating
a <t:document>, as that can obviously not be used from core.
>
> One new issue after moving the AddResource.java file and its
> dependencies to commons is that AddResourceTest uses some classes in
> myfaces-impl. I left AddResourceTest in tomahawk while we reach a
> solution for this.
>
> And of course, nothing has been decided, so thoughts and ideas welcome :-)
>
> Regards,
>
> Bruno
>
> [1] http://issues.apache.org/jira/browse/MYFACES-152
> [2] http://issues.apache.org/jira/browse/MYFACES-883
> [3] http://www.mail-archive.com/dev@myfaces.apache.org/msg07873.html
>
> 2006/1/22, Bruno Aranda <br...@gmail.com>:
> > Yes, you are right sorry. I first thought that only two classes were
> > involved, but after moving them my first priority was to stabilise
> > again the code base. I am going to explain everything now...
> >
> > 2006/1/22, Simon Kitching <sk...@apache.org>:
> > > On Sun, 2006-01-22 at 21:27 +0100, Bruno Aranda wrote:
> > > > As you might have realised, I am moving the AddResource stuff to
> > > > commons. I am doing this because it will help to solve some old bugs
> > > > we have (such as MYFACES-152). I am going to explain this in detail
> > > > when I stabilise again the codebase after this moves.
> > > >
> > > > Thanks for your patience,
> > >
> > > I was quite surprised to see the commit messages moving this class.
> > > Perhaps some discussion first might have been a good idea?
> > >
> > > Regards,
> > >
> > > Simon
> > >
> > >
> >
Re: Moving AddResource stuff to common
Posted by Bruno Aranda <br...@gmail.com>.
Ok, now that everything is stable again I am going to explain the
reasons. I was going to send a mail after moving the classes and
before doing anything else :)
With the current code, we cannot move f:view from where it is now, for
instance now we cannot include the head inside the jsf markup. This
could be solved moving the f:viewTag before the head, but this doesn't
not work in myfaces because in our ResponseWriterImpl we write stuff
(javascript for dummyForm, autoscroll...) when the viewTag is closed
(ResponseWriter.endDocument()). We should move that code out of
ResponseWriterImpl.endDocument() and put it in another place to be
renderer just before the closing </body> tag. Alongside we will fix
two important bugs (MYFACES-152 [1] and MYFACES-883 [2]) that hinders
us to use adf-faces and facelets together with myfaces.
We can use AddResource.java to write the stuff before the </body>. It
has been moved to commons because we should call to this class from a
base html renderer. Martin and I have been thinking that a good place
to call to AddResource could be the viewTag component, although there
are some issues against that as Jacob pointed out in a mail some time
ago [3]. Our proposal is to (1) to create a document tag (just like in
adf-faces) and call to AddResource from there and (2) also we could
also render it in the f:view tag if the document tag is not included
in the page. All this will solve those old bugs and we could have a
new document tag which may simplify things.
One new issue after moving the AddResource.java file and its
dependencies to commons is that AddResourceTest uses some classes in
myfaces-impl. I left AddResourceTest in tomahawk while we reach a
solution for this.
And of course, nothing has been decided, so thoughts and ideas welcome :-)
Regards,
Bruno
[1] http://issues.apache.org/jira/browse/MYFACES-152
[2] http://issues.apache.org/jira/browse/MYFACES-883
[3] http://www.mail-archive.com/dev@myfaces.apache.org/msg07873.html
2006/1/22, Bruno Aranda <br...@gmail.com>:
> Yes, you are right sorry. I first thought that only two classes were
> involved, but after moving them my first priority was to stabilise
> again the code base. I am going to explain everything now...
>
> 2006/1/22, Simon Kitching <sk...@apache.org>:
> > On Sun, 2006-01-22 at 21:27 +0100, Bruno Aranda wrote:
> > > As you might have realised, I am moving the AddResource stuff to
> > > commons. I am doing this because it will help to solve some old bugs
> > > we have (such as MYFACES-152). I am going to explain this in detail
> > > when I stabilise again the codebase after this moves.
> > >
> > > Thanks for your patience,
> >
> > I was quite surprised to see the commit messages moving this class.
> > Perhaps some discussion first might have been a good idea?
> >
> > Regards,
> >
> > Simon
> >
> >
>
Re: Moving AddResource stuff to common
Posted by Bruno Aranda <br...@gmail.com>.
Yes, you are right sorry. I first thought that only two classes were
involved, but after moving them my first priority was to stabilise
again the code base. I am going to explain everything now...
2006/1/22, Simon Kitching <sk...@apache.org>:
> On Sun, 2006-01-22 at 21:27 +0100, Bruno Aranda wrote:
> > As you might have realised, I am moving the AddResource stuff to
> > commons. I am doing this because it will help to solve some old bugs
> > we have (such as MYFACES-152). I am going to explain this in detail
> > when I stabilise again the codebase after this moves.
> >
> > Thanks for your patience,
>
> I was quite surprised to see the commit messages moving this class.
> Perhaps some discussion first might have been a good idea?
>
> Regards,
>
> Simon
>
>
Re: Moving AddResource stuff to common
Posted by Simon Kitching <sk...@apache.org>.
On Sun, 2006-01-22 at 21:27 +0100, Bruno Aranda wrote:
> As you might have realised, I am moving the AddResource stuff to
> commons. I am doing this because it will help to solve some old bugs
> we have (such as MYFACES-152). I am going to explain this in detail
> when I stabilise again the codebase after this moves.
>
> Thanks for your patience,
I was quite surprised to see the commit messages moving this class.
Perhaps some discussion first might have been a good idea?
Regards,
Simon