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/12/12 13:11:14 UTC
[GitHub] [myfaces] werpu opened a new pull request, #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
werpu opened a new pull request, #356:
URL: https://github.com/apache/myfaces/pull/356
hat to recreate the pull request because i had to rename the branch
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1311101287
Thank you. Alternatively, we could change it in the backend to generate {key:value} objects instead. I'm not sure what's most appropriate here.
Also I tested the file upload and the single upload via ajax is working now. However, the multiple file is not? I only saw one file encoded.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1335239311
conflicts fixed
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1326669169
Number of Unit Tests on the codebase now 177!
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1336971903
I was finally able to run the 2.2 TCK tests against our new codebase.
Following Tests are still failing (note, not all of them might
have a js issue as base):
[ERROR] Issue2041IT.testIssue2041:48
[ERROR] Issue2179IT.testDecodeException:57
[ERROR] Issue2179IT.testEncodeException:40
[ERROR] Issue2439IT.testUpdateAttributeNamedValue:41
[ERROR] Issue2636IT.testCommandLinksInRepeat:38
[ERROR] Issue2674IT.testProgrammaticAjaxBehavior:36
[ERROR] Issue2906IT.testCommandLinksInRepeat:40
[ERROR] Issue3097IT.testViewExpired1:45
[ERROR] Issue3473IT.testButtonOnlySubmitsOne:41
[ERROR] Issue3833IT.testUIRepeatStateSaving:56
[ERROR] Spec1296IT.testPartialResponseWriterOutsideFacesServlet:36
I will investigate them and fix them long the lines (also unit tests will be added wherever they are possible to cover the failing tests.
Will run the same against the old codebase later on.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1425458654
Closing this now, I have opened another pull request: https://github.com/apache/myfaces/pull/514
Please continue the documentation there
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "melloware (via GitHub)" <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416038482
I like that idea.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339396408
Btw. short update. Given I could not determine anything anymore in the Ajax codebase itself on my side from the ajax TCKs, I would say the code is merge ready.
I cannot do anything regarding the other failing tests, except for filing in bugs on both sides.
How about merging the code into main this week?
This would relieve me of the task of having to crossport every fix there is from 2.3 into main!
--
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] werpu closed pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu closed pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
URL: https://github.com/apache/myfaces/pull/356
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1301010056
For the duplicates, I click the button twice which then add the script / link elements each time. I saw Mojarra had map to avoid this problem. Could we do the same?
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1308521951
Hi I have isolated the problem and filed a bugreport:
https://issues.apache.org/jira/browse/MYFACES-4496
If anyone wants to look into this, self running example for demonstrating it is linked to a github project I just opened!
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310351363
Thanks for the metrics!
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1338988239
TCK Faces 23 Tests
**Spec1412**:
```java
button.click();
page.waitReqJs(Duration.ofMillis(60000));
assertTrue(page.isInPage("Success!"));
```
Fails... will investigate
**Spec1423**:
```java
assertTrue(page.findElement(By.id("scriptResult")).getText().trim().equals("addedViaInclude"));
```
fails
could be an issue on the client will investigate
Comments pending
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1351566221
Hi I have reopened the tck ajax pull request, this time against the proper branch https://github.com/jakartaee/faces/issues/1722
Lets see if they take it 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 pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1299148181
One item I found -- this code doesn't seem to handle `jakarta.faces.Resource` in ajax responses Such as
`<?xml version="1.0" encoding="UTF-8"?><partial-response id="j_id__v_0"><changes><update id="jakarta.faces.Resource"><![CDATA[<script src="/test-faces23-ajax-4466/jakarta.faces.resource/addedViaHead.js.xhtml?ln=spec1423"></script>]]></update>`.
Feel free to test spec1423.xhtml
[test-faces23-ajax.war.zip](https://github.com/apache/myfaces/files/9913931/test-faces23-ajax.war.zip)
Source code is from: https://github.com/jakartaee/faces/tree/4.0.1/tck/faces23/ajax
Here's our current code for the resource check: https://github.com/apache/myfaces/blob/89c747e85615e3f33265e664c8361789f38ea7db/api/src/main/javascript/META-INF/resources/myfaces/_impl/xhrCore/_AjaxResponse.js#L531-L535
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1299803155
Hi guys, I would be ready for merge, now... please complete the review and if you have objections please point them out for correction. If all goes well I can wrap everything up this week.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1309874497
I have an investigative issue to the reported HTMLUnit issue here https://github.com/werpu/jsfs_js_ts/issues/27
For now this is just documentation, I want to see whether HTMLUnit "chokes" on the code, which is very likely given we use a relatively modern browser baseline, and whether we can do something about it.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310479713
Not sure if we can proceed with our revamped codebase if the TCKs fail on a simple faces.js load because they use an engine which requires es5 (es5 definitely is not a base level per spec)... they definitely will pass if we compile against es5 as long as they just check for the interfaces to be there. If they run rountrip testing then they might fail as well.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1311285354
> Thank you. Alternatively, we could change it in the backend to generate {key:value} objects instead. I'm not sure what's most appropriate here.
>
I think it would make sense to detect both patterns, given it is easy to detect them on ecmascript level, and it is only a few locs extra it makes sense.
> Also I tested the file upload and the single upload via ajax is working now. However, the multiple file is not? I only saw one file encoded.
I have to recheck that, my testing example has two fileuploads in one form, but I only checked by uploading one file. I will come back to you once I know more.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1311425893
> Also I tested the file upload and the single upload via ajax is working now. However, the multiple file is not? I only saw one file encoded.
Works in my testcase:
```html
<h:form id="testForm" type="multipart/form-data" enctype="multipart/form-data">
<h:inputFile id="bla" value="#{fileupload.uploaded}">
</h:inputFile>
<h:inputFile id="bla3" value="#{fileupload.uploaded2}">
</h:inputFile>
<h:commandLink id="bla2" value="Upload" action="#{fileupload.doUpload}" type="button">
<f:ajax execute="@form" render="successfield" />
</h:commandLink>
<h:outputText id="successfield" value="#{fileupload.msg}"></h:outputText>
</h:form>
```
```java
@Named
@RequestScoped
public class Fileupload {
private Part uploaded;
private Part uploaded2;
private String msg = "";
public String getMsg() {
return msg;
}
public void setMsg(String msg) {
this.msg = msg;
}
public void doUpload() {
if(uploaded != null && uploaded2 != null) {
msg = "success";
}
}
...
<img width="789" alt="image" src="https://user-images.githubusercontent.com/530547/201306063-16ea533a-9d10-4597-bc84-85e1c735e7fd.png">
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1285746656
4487 is in, I had to alter the resource loader call chain slightly and have introduce one which does the mapping file referencing upon load time.
Please test properly whether this raises any issues.
This is probably my last big addition in the myfaces 4.0.0 jsf.js related codebase, unless something crawls up bugwise.
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1335169931
Looks like you have a couple of conflicts?
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339416689
Yikes OK then I will leave it up to @tandraschko @volosied what they want to do. Sounds like this could be tricky/messy.
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339506119
Hi, let me find out if there's been any progress or testing done with the Selenium TCK PR (1732) yet.
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416073238
if they are challenged and closed, please lets just close our JIRA issues
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1314908927
Command back, oamsubmit is not entirely fixed, I have to roll another small fix and another unit test.
the case myfaces.oam.submitForm('form1','doSubmit') of simple submits without parameters is not working atm.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1300789844
> @werpu Thanks for working on this!
>
> I tested it out, and it's improved. However, I still see some odd behavior.
>
> * The CSS stylesheets are applied (i.e background color changes), but the JS scripts aren't executed nor do I see network requests for them?
> * The other thing I noticed is that resources are applied to the HEAD more than once ( which I don't think is intended per the TCK)
>
> <img alt="image" width="800" src="https://user-images.githubusercontent.com/5934310/199529954-bb786461-47a7-46c9-bf65-5b9fa3b862d5.png">
> I tested it out, and it's improved. However, I still see some odd behavior.
>
> * The CSS stylesheets are applied (i.e background color changes), but the JS scripts aren't executed nor do I see network requests for them?
> * The other thing I noticed is that resources are applied to the HEAD more than once ( which I don't think is intended per the TCK)
Ok I will do another round of testing... no resources should not be applied more than once...
I have to check that and scripts should be executed. I will add additional tests and check where the problem is, thanks for testing. I will use your example now for testing (I thought the fix was easy enough to cover it with my unit tests only.
Well sloppyness bites back instantly.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1340725690
Hi I just noticed i ported from a rather old branch (4.0.1) there have been many bugfixes going into the TCK, so I will have te Re-port again and update the pull request accordingly. This will happen until sometime next week.
(we have a holiday tomorrow)
not a big deal but a little bit of work. With that I also can add my latest base framework which has a few smaller fixes over the PR I offered for the TCK.
(already added a comment, that they should postpone until next week for that), but nevertheless an info would be nice if they would be willing to add selenium based tck tests at least optionally.
So that we can test as well from the official tck codebase!
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1300673807
@werpu Thanks for working on this!
I tested it out, and it's improved. However, I still see some odd behavior.
- The CSS stylesheets are applied (i.e background color changes), but the JS scripts aren't executed nor do I see network requests for them?
- The other thing I noticed is that resources are applied to the HEAD more than once ( which I don't think is intended per the TCK)
<img width="800" alt="image" src="https://user-images.githubusercontent.com/5934310/199529954-bb786461-47a7-46c9-bf65-5b9fa3b862d5.png">
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1305207331
Ok... I also got a positive report in from Tobago... so @volosied if you can give the ok from your side, I would be ready to merge.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310011363
Found the issue, apparently html unit bundles Mozilla Rhino and the bundled version or any version (still not fully done with my research on that yet) does not support Ecmascript 2015 syntax. After compiling the files down to es5 I get a more meaningful error in my internal testing framework that the keyword class (explicit classes were introduced in es2015) is not allowed.
I checked on Mozillas side and found this:
https://mozilla.github.io/rhino/compat/engines.html
There are two possible fixes to move forward
a) We can return to an ES5 compile target, which is rather pointless given the apis we use, and might run into compatibility issues with html unit anyway (we use FormData elements XhrObject, querySelectorAll etc... which are APIs which might not exist on ES5 level browser implementations)
But nevertheless it is worth a shot, but my hopes are somewhat limited.
b) You guys can have a look to get the unit tests out of this ES5 level maybe move the tests to selenium where you can use different drivers or move to a client side testing framework. I know this is a little but much to ask. But as it seems HTMLUnit for now as testrunner seems to be a dead end as long as it uses Rhino and/or Rhino does not move forward.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1313725724
OamSubmit now is fixed, should work as expected, I am now covering both methods of parameters and added 5 new Unit Tests to cover this api. You now can either pass tuples or associative arrays:
https://github.com/apache/myfaces/pull/356/commits/4545045fa901d42bc862d7546e281ebf7d9adac3#diff-b7998247c1ec37ef6a46d490efa49d3b385ea436fc765ab3dd02f68485aab44b
You get the idea from the newly added tests!
@volosied let me know if this fixes your last remaining issue, without having to patch the ButtonRenderer!
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1314345459
@werpu Thank you for getting the array issue fixed! It looks good now.
And thanks again for looking into the html unit problem, too. Keep us updated.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1318783645
Hi short update, I got the two tck bugs in, have not had time to fix them today, I will fix them probably tomorrow. As I am busy atm porting the TCK tests to Webdriver. I have now about 50% of the mojarra JSF 2.2 TCK tests ported and have solved all missing pieces which HTMLUnit had but webdriver does not (aka request response data is not exposed by WebDriver, but you can solve it over the Chrome debugging engine)
So I am making good progress, to get the TCKs updated. I probably will have a pull request on the jakarta side ready sometime next week.
--
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] werpu closed pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu closed pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
URL: https://github.com/apache/myfaces/pull/356
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422966458
Please no immediate release, while the code has been way better tested than the old implementation, I want to give the community at least one RC to test the change out before going into release.
It is very uncommon to have such a big change in the last release candidate anyway.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1344294368
I will resolve the conflicts next week, they are related to my work on MYFACES-4526, MYFACES-4040
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416141251
As I said my cleanup is independent of anything... we can do it before or after. Preferrably before, though!
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1311973354
File uploads look good!
```
field: multipleSelection, name: test2.txt, size: 13
field: multipleSelection, name: test1.txt, size: 13
```
The f:param still isn't working for me...? However, I created a java fix here to use object notation over Arrays for params and it works with the patch.
```
diff --git a/impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlButtonRendererBase.java b/impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlButtonRendererBase.java
index ec08b2a60..588ba1eb2 100644
--- a/impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlButtonRendererBase.java
+++ b/impl/src/main/java/org/apache/myfaces/renderkit/html/base/HtmlButtonRendererBase.java
@@ -346,7 +346,7 @@ private StringBuilder addChildParameters(FacesContext context, List<UIParameter>
{
//add child parameters
StringBuilder params = SharedStringBuilder.get(context, SB_ADD_CHILD_PARAMETERS);
- params.append('[');
+ // params.append('[');
for (int i = 0, size = validParams.size(); i < size; i++)
{
@@ -398,13 +398,13 @@ else if (buff != null)
params.append(',');
}
- params.append("['");
+ params.append("{'");
params.append(name);
- params.append("','");
+ params.append("':'");
params.append(strParamValue);
- params.append("']");
+ params.append("'}");
}
- params.append(']');
+ // params.append(']');
return params;
}
```
--
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] bohmber commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by GitBox <gi...@apache.org>.
bohmber commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1396619947
I would like to see this merged
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1306750505
I have created an issue on my github side:
https://github.com/werpu/jsfs_js_ts/issues/26
I will fix the bugs there first and will keep an update on the status. Once everything is fixed, it is merged back into the pull request.
So if you want to keep track of things, you can follow there.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1311626485
Ok I have fixed both reported "issues"
The multiple files upload now works as expected:
Example:
```
<h:form id="testForm" type="multipart/form-data" enctype="multipart/form-data">
<h:inputFile id="bla" value="#{fileupload.uploaded3}" multiple="true">
</h:inputFile>
<h:commandLink id="bla2" value="Upload" action="#{fileupload.doUpload2}" type="button">
<f:ajax execute="@form" render="successfield" />
</h:commandLink>
<h:outputText id="successfield" value="#{fileupload.msg}"></h:outputText>
</h:form>
```
```java
@Named
@RequestScoped
public class Fileupload {
private List<Part> uploaded3;
...
public void doUpload2() {
if(uploaded3 != null) {
msg = "success";
}
}
```
Results in a success message and the number of incoming parts as well as their dats looks correct.
also if you want to pass outside passthrough data, the implementation follows now closely the spec:
```javascript
faces.ajax.request(element, null, {
execute: "input_1",
resetValues: true,
render: "@form",
params: {
pass1: "pass1",
pass2: "pass2"
}
});
```
It used to be:
```javascript
faces.ajax.request(element, null, {
execute: "input_1",
resetValues: true,
render: "@form",
pass1: "pass1",
pass2: "pass2"
});
```
Atm I support both formats, but I marked the second one (the old one in our code as deprecated to take it out once it is clear we do not have to support this anymore)
We also have this issue in our old code, this is clearly a spec breach from our side, so it needs to be fixed.
Will take care of that on monday, by adding the option 1 of the two to our old code and still allow the "faulty" way to pass those parameters.
Also to solve the issue for @volosied I added an option to allow the passthrough of parameters as tuple arrays:
For now only this construct (which is really bending the spec) works:
```javascript
faces.ajax.request(element, null, {
execute: "input_1",
render: "@form",
params: [["pass1", "pass1"],
["pass2", "pass2"]],
```
I hope this resolves the test problem from @volosied but in the end I would recommend to shift the parameter passing to an associative array instead of tuple arrays. Because the RI definitely will break on such a construct.
(I can take it out again, if not wanted, it is just 3 loc and a test)
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1312000272
Hi, as I wrote before, theoretically sending params as array is not covered by the spec, so very likely our implementation is wrong here anyway, so the patch seems valid to me, does it break anything outside the Ajax area (already out of work for today, so I can check it on monday)?
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1323821092
Ok I now have offered the pull request with the Selenium ported TCKs to the Jakarta project https://github.com/jakartaee/faces/pull/1732 they can take over from there.
I will commence now from tomorrow onwards on our codebase again, and finally will fix the 2 reported bugs.
Feel free to take the Selenium code from the tck for your own testing. The 2 reported bugs should be fixed soon!
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1344296561
@werpu I really appreciate all your work on 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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339403823
I agree this PR has been open for a long time.
--
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] milansie commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
milansie commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1308798406
hi, onerror was added here https://issues.apache.org/jira/browse/MYFACES-4415
@werpu you do not have `**jakarta**.faces.ENABLE_WEBSOCKET_ENDPOINT` enabled in your app. If you enable it, your demo will work..
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1322259356
Another short status update. I have ported now all 70 or so Ajax TCK tests to Selenium. I now am waiting on how they want the pull requests (selenium side by side to the old code or, as full replacement)
Once the pull request is out I will pick up the work on my MyFaces side again. Then we also should have a full working TCK testsuite for the new myfaces codebase.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1299923553
Ok just saw the comment, I will fix the issue.
Thanks for the feedback and BBohman who pointed me towards it.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1301040923
> For the duplicates, I click the button twice which then add the script / link elements each time. I saw Mojarra had map to avoid this problem. Could we do the same?
Yes definitely, another solution simply would be to check for elements with the same tags and src or hrefs. Given we use querySelectorall, the speed impact for doing this is neglectable.
I will add a fix and improved unit tests for this issue tomorrow, and I will also test the final code against your testcase, to make it watertight.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1303608273
Ok, thumbs up.
The tests posted here now work for me as well.
I noticed however that the tests themselves have a test which is not implementable.
Add resource to body the spec does not allow that ATM (javax.faces.Resource must be attached to the head)
results in:
a standard resource definition on the response protocol. atm. the resource response has no target attribute, so it is automatically attached to the head (not sure what the point of this test is)
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1307321421
I for now have fixed the file upload issue and the push.init issue (silent api change), the f:param issue was not reproducible.
I cannot test the push code atm because of a myfaces issue I face. I will isolate the problem in a self starting jar tomorrow and file a bugreport.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416023335
Hi following:
Whats the plan. first I need to merge the fixes we got from the server for TCK 790, then also my client fixes for 790 which I have under a different branch.
The question now is after that how we proceed. The TCKs pass as well as with the old codebase, but with the selenium version. If that is good enough for being able to say we are TCK compliant, then I am fine.
If not we have to think about another solution maybe running a 4.0 next branch!
On the TS side I want to do a cleanup of the XHRFormData the upcoming weeks, the aim is to reduce complexity (it has grown too complex for my liking and has a ton of simplification potential), but that is independent of the merge!
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416031969
what about:
- try to fix all TCK bugs (MYFACES-4517 is the only one left?)
- create a new Milestone release
- in meantime you can cleanup your code and we can merge it
- create a new Milestone with the new JS base
- some testing time
- final release
WDYT @volosied @melloware @pnicolucci ?
--
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] pnicolucci commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "pnicolucci (via GitHub)" <gi...@apache.org>.
pnicolucci commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422963496
I think an RC5 is the best bet to roll up all the changes then we can run everything against the TCK. Then do a 4.0.0 release.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422916486
All the changes are merged in, waiting for the final test of the Tobago people, TCK tests pass!
Enjoy the improved code. This was very likely the last big refactoring on my part before the final merge. The rest of the code is more or less in a state I can live with!
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339428091
Yes thats the reason why I started with the selenium port of the TCK to begin with, to get the tests out of the tech dead end they obviously have been for quite a while. Selenium simply is under active development and has strong corporate backing.
Whether the Jakarta is going to pick up the PR or not, good question, but so far I have not seen any significant feedback.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1303521479
Status update: Sorry it took longer, but given I discovered a race condition with a mix of script and script src= tag elements I had to overhaul the algorithm how we load script src= elements. We probably have the same issue in the old code.
The code now works on my side, I also added unit tests for the reported cases of double include and src files not being loaded (some test code is now externalized into test fixtures which was before inline)
The last test pending is the one against the attached war, I will do that now before giving the general ok. But if you want to start testing feel free not to wait for me.
If all goes well I then I will start the merge on monday.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1313201984
Question for me is, why are we were doing it this way in the first place. I have to check my old code on this (its been years so I do not remember if I have covered that case back then)
I need to run another investigation. The reason why I could not reproduce it in my tests, simply probably was, beause I was using a commandLink instead of a button.
I will dig a little bit deeper into this. Might be a bug in MyFaces which is stone old.
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1312138070
I'll do some testing, but it shouldn't break anything. We'll talk Monday!
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1308697053
cc @milansie as WebSockets was just refactored with this PR: https://github.com/apache/myfaces/pull/327
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1285073291
I will revisit the issue regarding the mapping files again. Atm we have solved that by adding a special extension which allows if mapped to the faces servlet to load the map file for faces-development.
Given I want mapping to be optional, I will revisit this issue again, I probably will provide a special servlet filter on the impl side, which adds the mapping file data automatically in the js file depending on the faces servlet setting, if wanted.
I will open a jira enhancement issue for this and will provide the code.
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1346474806
Says there are conflicts?
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416360022
Never mind, I am almost done with the cleanup, I just gave the Tobago guys the code for pretesting, will merge it back in probably around tuesday!
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1338956652
@volosied I have opened a public github repository with my work so far: https://github.com/werpu/tckworkbench
I also have solved the issue with the multi projects. Theoretically you can now run the tests from the main pom instead of going into the subprojects. Practically the 22 tests will fail, if you exclude those it will start to pass.
Java11 is required and so is a working chrome installation on the testing machine.
My test setup is:
I compile Myfyces from the pull request (or my local pr branch https://github.com/werpu/myfaces/tree/feature/MYFACES-4466) and then given it already is in the 4.0-SNAPSHOT namespace, just start the tests.
Whatever is registered under 4.0 snapshot on the local machine is picked up!
That way you also can run the tests against the current main (which was not my goal atm but it is possible).
I have not ported all tests over - due to time constraints, but there is a PR on the jakarta side which has all ported tests, so it is a matter of copy pasting the test over, I will add the tests on the fly, so expect a fuller coverage of the Ajax parts soon.
Re license, I cannot relicense the TCK, so the workbench is under EPL, however my framework code sitting on top of Selenium adding the missing apis HTMLUnit has for request and response handling and inspection, is relicensed under ASL2.0 because it is 100% written by me (you can mix EPL and ASL, but the end result will be ESL, the other way around is not possible to my knowledge, so making an ASL2 project out of such a mix is impossible, but it does not matter, we cannot integrate it in MyFaces anyway)
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310300622
As I said, sticking with html unit and downcompiling it for es5 might work or not, but my gut feeling is we will run into other issues of outdated and not implemented apis on the dom side of htmlunit.
We can give it a shot (turning the build down from es2015 is just a compile flag thanks to typescript)
but I also agree stepping down to es5 just because we rely on an outdated testing framework is basically the wrong way to fix it. But I also would not block such a decision, I just do not have too many hopes that fixes all problems.
In the end this is a community decision like so many other things.
--
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] pnicolucci commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
pnicolucci commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310397275
The Faces TCK also uses HTMLUnit as a test client. There does exist https://htmlunit.sourceforge.io/apidocs/com/gargoylesoftware/htmlunit/WebClientOptions.html#setThrowExceptionOnScriptError-boolean- which if that allows any of the tck tests failing due to a javascript error (according to HTMLUnit) then it would be possible to challenge those tests from a test client problem perspective. @volosied was going to test and see how many tck tests are hitting this and if that would solve the errors.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "werpu (via GitHub)" <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422924118
@volosied @tandraschko, I have yet to check the status, but it seems like the selenium based tests are now part of the official tck, how about a merge of this early next week?
So we can get finally the new codebase in for RC5?
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310527405
Websockets looks to be fixed, though. Thanks!
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310515902
@werpu
To follow up on the f:param problem, I think it's related to the `<f:ajax execute="@this" render="output" />`:
With f:ajax:
```
hello2: Hello from f:param
Hello World: Hello from f:param2
```
I also noticed the array was structured with curly brackets and `myfaces.ab` is called.
onclick="faces.util.chain(this, event,function(event){myfaces.ab(this,event,'action','form1:submitButton2','form1:submitButton2',**{'hello2':'Hello from f:param','Hello World':'Hello from f:param2'})})**
Without f:ajax:
```
0: hello2,Hello from f:param
1: Hello World,Hello from f:param2
```
Sqaure Brackets and `myfaces.oam.submitForm` is called instead:
onclick="return myfaces.oam.submitForm('form1','form1:submitButton2',null,**[['hello2','Hello from f:param'],['Hello World','Hello from f:param2']]**);"
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1332184612
Number of unit tests in codebase now 181, 4 new ones covering the view id namespacing (portlet scenario) same are as TCK-790
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1337899980
Hi I forked away the TCK codebase and use Selenium as test base for it running an Embeedded Tomcat (the original code uses html unit and glassfish)
I atm have not shared the fork yet on github, but, I will open a Github project tomorrow.
I atm use the 4.0 branch and the new scripts for testing, the fork, is only for the Ajax parts of the TCK.
I wanted originally to share it once I have ported all tests over and have fixed everything which I encounter on my side, but I can share the fork if wanted earlier so that you guys can pick it up.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1335154536
Rolled in another bunches of fixes coming from the old codebase for 4511, unit tests now 182, all pass, integration tests also pass, tck tests as well!
I consider it fixed, will roll pull requests for 2.3-Next and master as well
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1306315248
Thanks for the reports, I will tackle every single one of those issues tomorrow.
the f:param is clearly still an issue in the new form encoding code as for the rest I will have a deeper look.
Most if not all should be fixed tomorrow or the day after tomorrow.
Thanks for reporting the test results.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1283777807
I will squash the commits at the final merge into master, the so the accidentally referenced myfaces-4456 commits will be squashed into one single big commit with a myfaces-4466 reference.
Renaming atm is not possible anymore due to rebase problems.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1308715686
Cool thanks waiting for the merge then, after that I will check the issue again, and if fixed can close it.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1314985861
Fixed and 3 new testcases added which test for reduce submitForm patterns.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1361308093
Not sure what the problem atm is... the branches are in sync but github is giving me sync problems to my private fork
Lets wait until this is resolved.
Otherwise I need to refork before the final merge of this pr
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1416045482
MYFACES-4517 is a duplicate of [MYFACES-4425](https://issues.apache.org/jira/browse/MYFACES-4425)
This test was challenged and accepted: https://github.com/jakartaee/faces/issues/1757. Do you still want this fixed?
MYFACES-4489 was also challenged. However, since I worked on the previous flow issue, I might be able to find a 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] tandraschko commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422947482
+1 from my side
we could even release 4.0.0 - all tickets are done after this
wdyt @volosied @pnicolucci ?
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC4)
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1422957872
I discovered a selectOneRadio bug caused by one of my changes. I am working to fix it now (will make a JIRA soon).
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1300527603
I have fixed the reported resource processing, issue reported in
https://github.com/apache/myfaces/pull/356#issuecomment-1299148181
and also documented under
https://github.com/werpu/jsfs_js_ts/issues/25
I have added 3 tests, and also have covered one corner case where a resource theoretically (will never happen, but it would be possible from the protocol itself) could come in as embedded script or css.
Either way unit tests are in place now which test for exactly the issue reported and which cover the spec and the additional possible side behavior.
Implementation works on the unit tests.
Please also test against 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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1302330608
Short update... Still working on the issue. While fixing this one I noticed that script src elements in Ajax calls were loaded asynchronously thus causing race conditions for mixed script src and embedded script tags. Apparently a bug which we also have in the old implementation, but never reported. So I had to fix this area as well.
I have the fix now running, but broke one of my nonce tests with the new code (the final integration test fails which tests for one of the nonce corner conditions)
So I will fix this one as well, and after that run the code against the linked war. But for now it is getting late, and I am calling it a day.
Sorry for the delay.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1313505876
Addition: I just did a preliminary test against HTMLUnit with an es5 compiled version of our scripts. As expected HTMLUnit chokes, it throws in this case an error on perfectly valid code which is even es5. So it is not even the Dom it has started to choke on.
The best bet would be to challenge the TCK in this area to use a better engine for client side testing. It wont brake the RI codebase by for instance moving to webdrivers but would help us a lot.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1339405880
Yes the downside I see is, if we merge now we lose tck testability, until the tck gets updated to selenium. This might be a problem for companies requiring full TCK compliance (IBM comes to my mind)
We might have to go the extra mile and make both implementations switchable...
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1283757935
@melloware I just had to rename the branch, I had accidentally added the wrong jira issue reference.
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1306143241
@werpu
Wanted to give you a quick update. Most tests are passing, but I see a few issues
1) f:param doesn't to be properly sent. For example:
```
<h:commandButton id="submitButton" value="Click here" action="#{elImplicitObjectBean.execute}">
<f:param name="message" value="Hello World"/>
</h:commandButton>
```
The code above sends the following in the form data: `0 "message,Hello+World"`.
```
0 "message,Hello+World"
form1_SUBMIT "1"
jakarta.faces.ViewState "N2RjNThlYTRlZjExNjJiODAwMDAwMDAz"
jakarta.faces.ClientWindow "10tijvumt"
form1:_idcl "form1:submitButton"
```
However, previously (working case), `message "Hello+World"` was sent.
```
form1_SUBMIT "1"
jakarta.faces.ViewState "ZjAyNTllMDgwODcxZTFlOTAwMDAwMDA0"
jakarta.faces.ClientWindow "-uveyxftzi"
message "Hello+World"
form1:_idcl "form1:submitButton"
```
This causes our app to fail since ExternalContext's getRequestParameterMap() cannot find "message".
2) Clicking "Click Here" on `ELImplicitObjectsViaCDI/index.xhtml` open up a new tab, but this shouldn't happen. See apps and source.
3) Our Websocket Listener's onOpen call is called during the page loading, but it should only occur on the button click.
<img width="490" alt="image" src="https://user-images.githubusercontent.com/5934310/200405665-cb383264-3a5c-44c8-9eae-47bee0c85f15.png">
URL is: `WebSocket/faces40/OpenCloseWebSocketTest.jsf`
4) Related to the websocket issue above, pressing the onClose button (to close websocket channel) encounter this exception:
```
Uncaught TypeError: e.components[s].onclose is not a function
onclose http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
onclose http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
bindCallbacks http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
open http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
open http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
open http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
init http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
init http://localhost:9080/WebSocket/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces:2
<anonymous> http://localhost:9080/WebSocket/faces40/OpenCloseWebSocketTest.jsf:20
```
5) Our html unit library is encoutnering a script exception? I don't don't see this in the browser, so I think it's just our library. We'll try to see if it goes away with a version update. I validated the JavaScript syntax myself (compressed and uncompressed files) and I didn't find anything. However, could you also double check? Exception isn't clear where this "error" is.
```
com.gargoylesoftware.htmlunit.ScriptException: syntax error (http://localhost:8010/CDIManagedProperty/jakarta.faces.resource/faces.js.jsf?ln=jakarta.faces#2)
```
Source code can be found:
https://github.com/OpenLiberty/open-liberty/tree/integration/dev/com.ibm.ws.jsf.2.3_fat/test-applications/ELImplicitObjectsViaCDI.war
https://github.com/OpenLiberty/open-liberty/tree/integration/dev/com.ibm.ws.jsf.2.3_fat/test-applications/WebSocket.war
But I've also attached the apps.
[Apps.zip](https://github.com/apache/myfaces/files/9955133/Apps.zip)
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1305224968
Btw. theoretically we also could replace the codebase or add the next gen codebase to 2.3 (as second loading option) the codebase can work in both namespaces, via frontend shims)
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1306168003
Another thing I just noticed: The ajax file submissions (single / multiple) doesn't seem to work.
https://github.com/jakartaee/faces/tree/master/tck/faces40/inputFile
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1308981736
> hi, onerror was added here https://issues.apache.org/jira/browse/MYFACES-4415
>
> @werpu you do not have `**jakarta**.faces.ENABLE_WEBSOCKET_ENDPOINT` enabled in your app. If you enable it, your demo will work..
Uuups yes, indeed, i was using the javax.namespace, the error now is gone if i move over to the other one.
I will wrap up my work on the push request tomorrow, will close the bug as invalid. Thanks for pointing it out.
--
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] melloware commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
melloware commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310293842
My two cents not sure if it helps or not but...
1. I don't want to go back to ES5 compatibility just to make the HTMLUnit tests work.
2. Upgrading from HtmlUnit to Selenium would be a MUCH bigger change.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1310573765
@volosied thanks for the additional input, I will look into this tomorrow again.
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1315489032
I have now added a TCK challenge regarding the usage of HTMLUnit. The question is, can we provide a port of the existing Ajax TCKs to Selenium without "staining ourselves, by having worked on non ASL code"?
The situation is more relaxed now that the TCK is not closed source anymore, but the issue of an incompatible license persists.
I have asked the question as legal question in the mailing list, but have not gotten any answer, so I am trying here.
If the answer is ok, i can fork the RI, to prepare a pull request, I would start to work immediately on the Webdriver TCK pull request.
--
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 #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
volosied commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1337745728
Thanks for testing the codebase against the 2.2 TCK. How did you get it set up to run against MyFaces? I'm curious as my team's only option is testing through Open-Liberty's faces-4.0 feature.
Did your tests use htmlunit or the Selenium?
Which version of the TCK did you use? 4.0.1 or the main branch? We've made numerous challenges so far, and a few of the tests have already been updated (particularly - removing Mojarra specific checks, such as [Issue2674IT](https://github.com/jakartaee/faces/commit/069d2c9b190892af984e59b83d2f8ccd0b61599f)).
Lastly, one issue in the list that I noticed is [Issue2439IT#testDisabledBehaviors](https://github.com/jakartaee/faces/blob/master/tck/faces22/ajax/src/test/java/ee/jakarta/tck/faces/test/servlet30/ajax/Issue2439IT.java#L43). I had spent some time looking at it previously (but haven't made any jira yet) and I agree it's a Java-side issue. We don't register the handler at all due to the return on this line: https://github.com/apache/myfaces/blob/main/impl/src/main/java/org/apache/myfaces/view/facelets/tag/faces/core/AjaxHandler.java#L391 I removed the return and test still failed because it rendered `onchange=""`. We'd have to include a empty string check to avoid writing on change. Maybe there's another way to fix it. Also, the comment just above the return refers to the example below in the JSF 2.0 spec. Our impl thinks one is an inner and the other an outer handler.
```
“For this example, the inner <f:ajax/> would apply to “button1”. The outer (wrapping) <f:ajax> would not be applied, since it is the same type of submitting behavior (AjaxBehavior) and the same event type (action). For this example, since the event types are the same, the inner <f:ajax> event overrides the outer one.”
<f:ajax event=”click”>
<h:inputText id=”text1”>
<f:ajax event=”click”/>
</h:inputText>
</f:ajax>
```
I'm not sure if we should challenge this test or try to fix our implementation? Firstly, Issue2439IT was created due to a syntax error. Secondly, does it make sense to have two f:ajax tags for h:inputText for the same on change event?
--
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] werpu commented on pull request #356: Feature/myfaces 4466 faces.js reimplementation (target 4.0.0 RC3)
Posted by GitBox <gi...@apache.org>.
werpu commented on PR #356:
URL: https://github.com/apache/myfaces/pull/356#issuecomment-1299924085
> One item I found -- this code doesn't seem to handle `jakarta.faces.Resource` in ajax responses Such as `<?xml version="1.0" encoding="UTF-8"?><partial-response id="j_id__v_0"><changes><update id="jakarta.faces.Resource"><![CDATA[<script src="/test-faces23-ajax-4466/jakarta.faces.resource/addedViaHead.js.xhtml?ln=spec1423"></script>]]></update>`.
>
> Feel free to test spec1423.xhtml
>
> [test-faces23-ajax.war.zip](https://github.com/apache/myfaces/files/9913931/test-faces23-ajax.war.zip)
>
> Source code is from: https://github.com/jakartaee/faces/tree/4.0.1/tck/faces23/ajax
>
> Here's our current code for the resource check:
>
> https://github.com/apache/myfaces/blob/89c747e85615e3f33265e664c8361789f38ea7db/api/src/main/javascript/META-INF/resources/myfaces/_impl/xhrCore/_AjaxResponse.js#L531-L535
Hi I will have a look, thanks for reporting it.
--
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