Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespaces on inline HTML tags causes the component tree to be malformed #4281

Closed
ren-zhijun-oracle opened this issue Sep 18, 2017 · 16 comments
Assignees
Labels

Comments

@ren-zhijun-oracle
Copy link
Contributor

If we have a fragment like

<h:body>
  <c:if ...>
    <p xmlns="http://www.w3.org/1999/xhtml">Some text</p>
  </c:if>
  <h:button ... />
</h:body>

(The tags and namespace do not matter - so long as an HTML tag with a namespace declaration exists on the page, this error will occur)

The expected tree should have a structure like:

h:body
+ c:if
  - p
+ h:button

Instead it has a structure like:

h:body
+ c:if
  - p
  - h:button
@ren-zhijun-oracle
Copy link
Contributor Author

@BalusC Commented
Reproduced.

@ren-zhijun-oracle
Copy link
Contributor Author

@Toberumono Commented
I just realized that the pull request I made to fix this isn't listed here. It's #4282.

@ren-zhijun-oracle
Copy link
Contributor Author

@edburns Commented
Please see this important message regarding community contributions to
Mojarra.

https://javaee.groups.io/g/jsf-spec/message/30

Also, please consider joining that group, as that group has taken the
place of the old [email protected] mailing list.

Thanks,

Ed Burns

@ren-zhijun-oracle
Copy link
Contributor Author

@dmschlot Commented
Great to see there is a pull request already to fix this. This is a large bug for us and causing me to run a forked version of JSF.

All my HTML is valid XML, and XML serializers do not always allow you to control where namespaces are added. Inline SVGs are common enough that I feel it is a pretty good example of why this should be fixed. (Searching Google you'll find half the examples out there have xmlns on the inline svg.)

@BalusC Are you able to accept his pull request? If not who should I message?

@ren-zhijun-oracle
Copy link
Contributor Author

@Toberumono
Copy link

Toberumono commented Jan 7, 2020

The issue does not appear to have been fixed, and I cannot find an equivalent PR for the one that I made in the old repository (https://github.com/javaserverfaces/mojarra/pull/4282). Is there something that I need to or can do in order for my proposed fix to be included in the new repository?

@EvanKnowles
Copy link

Any progress on this? Accidentally reproduced this one again today.

@Toberumono
Copy link

Toberumono commented Jan 25, 2022

@EvanKnowles Unfortunately, my PR has been completely ignored since I provided it. Also, it appears that same PR was lost when the repository was transferred.
I don't have time to recreate the PR, but the relevant file should not have significantly changed. If you are comfortable running a fork, all of the changes you need are here: Toberumono/mojarra@8a1e001
Edit: The code has changed slightly. The change needs to be made 39 lines earlier in the file (starting on line 300).

@EvanKnowles
Copy link

@Toberumono No chance of running a fork this side unfortunately, thanks for the info though.

@mnriem
Copy link
Contributor

mnriem commented Feb 19, 2022

Relabeling as 2.3 as folks are saying it is an issue in 2.3 releases as well. Note that attaching a reproducer using only Mojarra will make it easier for the maintainers to address the issue.

@mnriem mnriem added 2.3 and removed pre-2.3 labels Feb 19, 2022
@BalusC
Copy link
Contributor

BalusC commented May 29, 2022

Still reproducible in 2.3.17. The long lost PR is here: javaee/mojarra#4282

@BalusC
Copy link
Contributor

BalusC commented May 29, 2022

I've verified that the original fix is working. I'll run the entire 2.3 test set first before prepping a new PR.

@Toberumono
Copy link

The same issue applies to JSF 3.0 as well. Should I make a new PR or will this also get merged into JSF 3.x? The fix is identical for 3.x, if that helps.

@BalusC
Copy link
Contributor

BalusC commented May 29, 2022

I'll upmerge, no worries. The entire 2.3 test set has passed, so I'll prep PR.

@Toberumono
Copy link

Actually, I have another PR from the old repository. I'm not sure if it was ever considered - it might count as a new feature.
It's this one: javaee/mojarra#4275

arjantijms added a commit that referenced this issue Jun 2, 2022
Fix #4281: corrupted component tree when using namespaced HTML elements
@BalusC BalusC closed this as completed in 12bd2e2 Jun 25, 2022
@melloware
Copy link
Contributor

Just an FYI this fix broke a scenario in #5140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants