Comments inline (towards the end)
Ken Paulsen wrote:
>
>
> Ryan Lubke wrote:
>> Ken Paulsen wrote:
>>>
>>> Ryan,
>>>
>>> Is there any reason that you put this in the renderView() method of
>>> the viewHandler as opposed to the createView() method? I believe
>>> all requests for images will end up going through createView(), not
>>> restoreView()... this gets called earlier, and will not get called
>>> for "post-back" type requests (so you won't have to waste time
>>> checking the mimetype for those requests).
>> I'll need to revisit - there was a reason that I did it that way, but
>> I can't recall at the moment.
>>>
>>> Also, while this helps serve images... it doesn't do anything for
>>> .pdfs or any other "resources" that might be referenced from a jsf
>>> page. It will also interfere with anyone that chooses a page name
>>> in an extension that maps to an image mime type. While this isn't
>>> extremely likely it is more restrictive... perhaps you want to
>>> explicitly specify which image types are supported by this (vs.
>>> letting the container decide).
>> You're right it doesn't support all binary formats. My main concern
>> was lessening the
>> impact of relative URIs with graphicImage.
> What does MyFaces do? I would worry that once you start supporting
> one type, you're going to have to start supporting more... With images
> alone, you're now dedicating the following file extensions to not be
> processed by JSF:
>
> .gif
> .ief
> .jpeg
> .jpg
> .jpe
> .pcx
> .png
> .svg
> .svgz
> .tif
> .tiff
> .djvu
> .djv
> .wbmp
> .ras
> .cdr
> .pat
> .cdt
> .cpt
> .ico
> .art
> .jng
> .bmp
> .psd
> .pnm
> .pbm
> .pgm
> .ppm
> .rgb
> .xbm
> .xpm
> .xwd
>
> And that's just my machine, it can vary. I recommend you pick a
> subset of the images and not let the server decide. Also, I recommend
> that you allow this subset to be overridden, this would allow other
> resource types to be handled as well (just saw a more recent email
> from you where you said the same thing).
>>> I agree this is an improvement... but it's an improvement on an
>>> error condition. The user specified the relative URL incorrectly.
>> I wouldn't say it's a user error. This worked fine in 1.1 due to how
>> the view handling worked in 1.1.
>> Since we buffer and post process in 1.2, it was an unexpected change
>> in behavior (reported by two
>> internal groups within sun). There is nothing in the spec that says
>> you shouldn't use relative URIs.
> True, but "relative" uri shouldn't be targetted at the FacesServlet,
> right? I don't recall anything in the spec that says the FacesServlet
> should process resource requests (perhaps it's in there and I missed
> it). The mistake is that the uri still includes the "/faces" (or
> whatever the prefix mapping is). An image component (or any other
> component that interacts w/ a resource) should prevent this from
> happening for their users.
>>> There are api's in JSF to obtain the correct URL. Components
>>> should make use of these to do this automatically. Users should use
>>> components to do this. Or... use extension mapping which doesn't
>>> have this problem.
>> Right, but it may be difficult to move to extension mapping (in the
>> tools case where all projects are generated
>> using prefix mapping).
> The components themselves should hide this so they don't have to. I
> would hope tools give the user's their users the choice... tools
> should help the user, not tell them what to do.
>
> I think it's a good feature... but unless the spec states otherwise, I
> think it's above and beyond what is required of the server. I'm not
> against the feature so long as it is configurable in case it causes
> problems (it's not likely to anyway).
You make some good points.
Would you mind logging an enhancement request to track this?
>
> Ken
>>>
>>> Ken
>>>
>>>
>>> Jason Lee wrote:
>>>> r=jdlee
>>>>
>>>> -----
>>>> Jason Lee, SCJP
>>>> Programmer/Analyst
>>>> http://www.iec-okc.com
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Ryan.Lubke_at_Sun.COM [mailto:Ryan.Lubke_at_Sun.COM] Sent: Monday,
>>>>> October 16, 2006 3:27 PM
>>>>> To: dev_at_javaserverfaces.dev.java.net
>>>>> Subject: [REVIEW - PART 2] Avoid full lifecycle processing for
>>>>> images when using prefix mapping
>>>>>
>>>>> I've attached a follow up change bundle to this issue. It turns
>>>>> out that the PhaseListener approach isn't going to work (causes
>>>>> issues with Shale). I've instead moved the PL logic into
>>>>> ViewHandlerImpl.renderView().
>>>>>
>>>>>
>>>>>
>>>>> Ryan Lubke wrote:
>>>>>
>>>>>> Avoid full lifecycle processing with the FacesServlet is
>>>>> prefix mapped
>>>>>> and images are defined using relative URIs.
>>>>>>
>>>>>>
>>>>>> SECTION: Modified Files
>>>>>> ----------------------------
>>>>>> M src/com/sun/faces/jsf-ri-config.xml
>>>>>>
>>>>>> A src/com/sun/faces/lifecycle/ImagePhaseListener.jav
>>>>>>
>>>>>>
>>>>>> SECTION: Diffs
>>>>>> ----------------------------
>>>>>> Index: src/com/sun/faces/jsf-ri-config.xml
>>>>>> ===================================================================
>>>>>> RCS file:
>>>>>>
>>>>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/jsf-ri-config.xm
>>>>>
>>>>>
>>>>>> l,v
>>>>>> retrieving revision 1.69
>>>>>> diff -u -r1.69 jsf-ri-config.xml
>>>>>> --- src/com/sun/faces/jsf-ri-config.xml 11 May 2006 18:48:03
>>>>>> -0000 1.69
>>>>>> +++ src/com/sun/faces/jsf-ri-config.xml 11 Oct 2006
>>>>> 00:55:38 -0000
>>>>>
>>>>>> @@ -167,9 +167,10 @@
>>>>>>
>>>>> <converter-class>javax.faces.convert.EnumConverter</converter-class>
>>>>>
>>>>>> </converter>
>>>>>> - <!-- Add our InitializingPhaseListener -->
>>>>>> + <!-- Add our implementation specific PhaseListeners -->
>>>>>> <lifecycle>
>>>>>>
>>>>> <phase-listener>com.sun.faces.lifecycle.ELResolverInitPhaseListener</p
>>>>>
>>>>>
>>>>>> hase-listener>
>>>>>>
>>>>>> +
>>>>> <phase-listener>com.sun.faces.lifecycle.ImagePhaseListener</phase-list
>>>>>
>>>>>
>>>>>> ener>
>>>>>>
>>>>>> </lifecycle>
>>>>>>
>>>>>> <!-- Configure Standard Validators -->
>>>>>>
>>>>>>
>>>>>> SECTION: New Files
>>>>>> ----------------------------
>>>>>> SEE ATTACHMENTS
>>>>>>
>>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>> --
>>>>>>
>>>>>>
>>>>>>
>>>>> ---------------------------------------------------------------------
>>>>>
>>>>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>>>>> For additional commands, e-mail:
>>>>> dev-help_at_javaserverfaces.dev.java.net
>>>>>
>>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>>>
>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>