myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <skitch...@apache.org>
Subject Re: Moving AddResource stuff to common
Date Sun, 22 Jan 2006 23:42:55 GMT
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 <brunoaranda@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 <skitching@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
> > >
> > >
> >


Mime
View raw message