You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by GitBox <gi...@apache.org> on 2022/10/12 02:39:20 UTC
[GitHub] [myfaces] volosied opened a new pull request, #338: MYFACES-4477 Fix Programmatic Views
volosied opened a new pull request, #338:
URL: https://github.com/apache/myfaces/pull/338
https://issues.apache.org/jira/projects/MYFACES/issues/MYFACES-4477
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
tandraschko commented on PR #338:
URL: https://github.com/apache/myfaces/pull/338#issuecomment-1281927134
yeah but you removed the "view exists" check, which is there by design. So we should run all integration tests to get sure, it still works. we have many viewid-mapping integration tests (not sure if they run currently because of jakarta <> arquillian?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r992929555
##########
impl/src/main/java/org/apache/myfaces/application/ViewIdSupport.java:
##########
@@ -482,14 +482,7 @@ protected String handleSuffixMapping(FacesContext context, String requestViewId)
}
}
- // Otherwise, if a physical resource exists with the name requestViewId let that value be viewId.
- if (isViewExistent(context,requestViewId))
- {
- return requestViewId;
- }
-
- //Otherwise return null.
- return null;
+ return requestViewId;
Review Comment:
Physical page doesn't exist, so it doesn't make sense to check if the view exists. Hope this doesn't cause any errors, but the build was successful. I'll see what the TCK says.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r999893060
##########
impl/src/test/java/org/apache/myfaces/view/facelets/impl/ProgrammaticViewBean.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.view.facelets.impl;
+
+
+import static jakarta.faces.application.StateManager.IS_BUILDING_INITIAL_STATE;
+
+import java.io.IOException;
+import java.util.List;
+
+import jakarta.el.ELContext;
+import jakarta.el.ExpressionFactory;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.faces.annotation.View;
+import jakarta.faces.component.UIComponent;
+import jakarta.faces.component.UIOutput;
+import jakarta.faces.component.html.HtmlBody;
+import jakarta.faces.component.html.HtmlOutputText;
+import jakarta.faces.context.FacesContext;
+import jakarta.faces.view.facelets.Facelet;
+
+/* Modelled off of corresponding TCK test */
+@View("/test.xhtml")
+@ApplicationScoped
+public class ProgrammaticViewBean extends Facelet {
+
+ @Override
+ public void apply(FacesContext facesContext, UIComponent root) throws IOException {
+
+ if (!facesContext.getAttributes().containsKey(IS_BUILDING_INITIAL_STATE)) {
+ return;
+ }
+
+ ComponentBuilder components = new ComponentBuilder(facesContext);
+ ELContext elContext = facesContext.getELContext();
+ ExpressionFactory expressionFactory = facesContext.getApplication().getExpressionFactory();
+
+ List<UIComponent> rootChildren = root.getChildren();
+
+ UIOutput output = new UIOutput();
+ output.setValue("<html xmlns=\"http://www.w3.org/1999/xhtml\">");
+ output.getAttributes().put("escape", false);
Review Comment:
manually setting escape to false
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
tandraschko commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r999278095
##########
impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlTextRendererBase.java:
##########
@@ -142,8 +142,8 @@ protected void renderOutput(FacesContext facesContext, UIComponent component) th
}
else
{
- //default is to escape
- escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, true);
+ //default is NOT to escape (Changed true -> false, Myfaces-4477)
+ escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
Review Comment:
isnt this a bug security flaw?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #338:
URL: https://github.com/apache/myfaces/pull/338#issuecomment-1284526273
``` [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.25 s - in org.apache.myfaces.view.facelets.impl.ProgrammaticViewTest```
Addressed Thomas' comments. Merging code in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r999894484
##########
impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlTextRendererBase.java:
##########
@@ -142,8 +142,8 @@ protected void renderOutput(FacesContext facesContext, UIComponent component) th
}
else
{
- //default is to escape
- escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, true);
+ //default is NOT to escape (Changed true -> false, Myfaces-4477)
+ escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
Review Comment:
I've removed the changes and updated the test to not escape the <html> elements.
I will bring this escaping issue up to the TCK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r992928736
##########
impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlTextRendererBase.java:
##########
@@ -143,7 +143,7 @@ protected void renderOutput(FacesContext facesContext, UIComponent component) th
else
{
//default is to escape
- escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, true);
+ escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
Review Comment:
Update comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #338:
URL: https://github.com/apache/myfaces/pull/338#issuecomment-1282775721
I'm not sure how to run the arquillian tests ( are there any?)
However, I've refactored this change to keep the "view exists" check. I now check for the facelet via the CDIUtils in the isViewExistent method.
Let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r992928736
##########
impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlTextRendererBase.java:
##########
@@ -143,7 +143,7 @@ protected void renderOutput(FacesContext facesContext, UIComponent component) th
else
{
//default is to escape
- escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, true);
+ escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
Review Comment:
todo: Update comment since default is now changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] tandraschko commented on pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
tandraschko commented on PR #338:
URL: https://github.com/apache/myfaces/pull/338#issuecomment-1280898768
please try to get the integrationtests running
we have some tests for the viewId mapping stuff, which must not fail after this commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #338:
URL: https://github.com/apache/myfaces/pull/338#issuecomment-1281406050
@tandraschko I've added a test to verify the programmatic facelet works. TCK also passes with these change. If no other concerns, please merge this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r992929011
##########
impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java:
##########
@@ -438,6 +438,22 @@ private DefaultFacelet _createCompositeComponentMetadataFacelet(URL url)
public Facelet getViewMetadataFacelet(FacesContext facesContext, String uri)
throws IOException
{
+ Boolean isManagedFacelet = _managedFacelet.get(uri);
+ if (isManagedFacelet == null || isManagedFacelet)
+ {
+ Facelet facelet = null;
+ if (ExternalSpecifications.isCDIAvailable(facesContext.getExternalContext()))
+ {
+ BeanManager bm = CDIUtils.getBeanManager(facesContext);
+ facelet = CDIUtils.get(bm, Facelet.class, true, View.Literal.of(uri));
+ }
+ _managedFacelet.put(uri, facelet != null);
+ if (facelet != null)
+ {
+ return facelet;
+ }
+ }
+
Review Comment:
Code from Thomas' initial fix for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied commented on a diff in pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied commented on code in PR #338:
URL: https://github.com/apache/myfaces/pull/338#discussion_r992928736
##########
impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlTextRendererBase.java:
##########
@@ -143,7 +143,7 @@ protected void renderOutput(FacesContext facesContext, UIComponent component) th
else
{
//default is to escape
- escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, true);
+ escape = AttributeUtils.getBooleanAttribute(component, ComponentAttrs.ESCAPE_ATTR, false);
Review Comment:
Update comment since default is now changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [myfaces] volosied merged pull request #338: MYFACES-4477 Fix Programmatic Views
Posted by GitBox <gi...@apache.org>.
volosied merged PR #338:
URL: https://github.com/apache/myfaces/pull/338
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org