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.

Sign up for GitHub

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

Jump to bottom

id attribute is missing in vdl of h:head and h:body #1760

Closed
BalusC opened this issue Dec 9, 2022 · 8 comments
Closed

id attribute is missing in vdl of h:head and h:body #1760

BalusC opened this issue Dec 9, 2022 · 8 comments
Labels
mojarra-implemented API issue implemented by Mojarra myfaces-implemented API issue implemented by MyFaces
Milestone
4.1

Comments

@BalusC
Copy link
Member

BalusC commented Dec 9, 2022

Observed in #1754

It was introduced in JSF 2.2 VDL as part of HTML5 work. But these attributes vanished in JSF 2.3 VDL.
@BalusC
Copy link
Member Author

BalusC commented Dec 13, 2022 •
edited

For the record, this was the ticket wherein it was introduced #1100 and this was the associated commit javaee/mojarra@84332c3

It appears tag doc was only added to a file named standard-html-renderkit-base.xml and not to html_basic.taglib.xml while it should have been kept in sync. And then the latter file was used for vdl doc generation from JSF 2.3 onwards during code deduplication efforts.

And then we can see in the commit that only Mojarra's HeadRenderer has been adjusted to output the ID attribute at any case when output mode is HTML5, even when it is not at all specified by the markup. This is not correct. Mojarra's BodyRenderer already rendered ID attribute since eclipse-ee4j/mojarra#2413, however this one did not check if the output mode is HTML5.

So the following adjustments need to be made:

  • in Faces spec, add back ID attribute to vdl doc of h:head and h:body and explicitly state that this is only used when HTML5 output mode is used.
  • in Mojarra impl, make sure the HeadRenderer outputs ID for HTML5 output mode only and only if it is actually specified in markup, i.e. when getId() returns non-null, rather than blindly rendering getClientId().
  • in Mojarra impl, make sure the BodyRenderer outputs ID only when HTML5 output mode is used, as originally intended.

BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Dec 13, 2022
@BalusC
https://github.com/jakartaee/faces/issues/1760
ba403c1 
- add back id attribute to VDL of h:head and h:body
- fix HeadRenderer to only output it when actually specified
- fix BodyRenderer to only output it when HTML5 doctype is used
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Dec 13, 2022
@BalusC
Revert "jakartaee/faces#1760 - add back id attribute to VDL of h:head…
baf7998 
… and h:body - fix HeadRenderer to only output it when actually specified - fix BodyRenderer to only output it when HTML5 doctype is used"

This reverts commit ba403c1.
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Dec 13, 2022
@BalusC
Revert "Revert "jakartaee/faces#1760 - add back id attribute to VDL o…
9c44881 
…f h:head and h:body - fix HeadRenderer to only output it when actually specified - fix BodyRenderer to only output it when HTML5 doctype is used""

This reverts commit baf7998.
@BalusC BalusC mentioned this issue Dec 13, 2022
Merged
@BalusC
Copy link
Member Author

BalusC commented Dec 13, 2022

Accidentally committed directly into master without PR, hence double revert via PR.

BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Dec 13, 2022
@BalusC
https://github.com/jakartaee/faces/issues/1760
ed46b4b 
Fix failing BodyRendererTest
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Dec 13, 2022
@BalusC
https://github.com/jakartaee/faces/issues/1760
89cb7e0 
HeadRenderer encodeBegin complexity was already removed since
introduction of new Doctype interface, so it can now be tested
@BalusC BalusC added this to the 4.1 milestone Dec 17, 2022
@tandraschko tandraschko added the myfaces-implemented API issue implemented by MyFaces label Feb 25, 2023
@BalusC BalusC added the mojarra-implemented API issue implemented by Mojarra label Jul 22, 2023
@BalusC BalusC closed this as completed Jul 22, 2023
@BalusC BalusC mentioned this issue Nov 19, 2023
Closed
@volosied
Copy link
Contributor

@tandraschko Can you point where this was implemented in MyFaces? I'm not seeing any ids rendered in RC1 when Doctype HTML 5 is used.

@tandraschko
Copy link

@volosied can't remember
but if you add a PR with a test, i could have a look

@volosied
Copy link
Contributor

volosied commented Feb 23, 2024 •
edited

@BalusC I'm seeing that the id attribute on the head element is no longer created on the namespacedView TCK app with 4.1? Is this intended?

https://github.com/jakartaee/faces/tree/694b38430ddd9ee9ce6e8670c026739c1b3c0721/tck/faces23/namespacedView

Edit: I'm just not seeing the scenario where id would be written on head /body?

@BalusC
Copy link
Member Author

BalusC commented Mar 5, 2024 •
edited

@volosied: id is only rendered when id is explicitly specified. Neither the original nor the new spec say anywhere that it should always be rendered even when unspecified.

Is it causing problems in namespaced views? If so, then we probably need to take a second look there and redefine the spec to explicitly mention namespaced views (like as here and there done, ajax e.g.).

@volosied
Copy link
Contributor

volosied commented Mar 7, 2024

Sorry I think I confused myself. I see what you mean now. However, I have a follow up question -- the ids should only be generated when HTML5 is specified, right?

I think there might be a bug in Mojarra? I should not expect the ID to be generated in the facelet below; however, I still see the ids rendered.

Could you double check yourself?

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="jakarta.faces.html">
    <h:head id="abc">
        <title>test</title>
    </h:head>
    <h:body id="def">
        test
    </h:body>
</html>

@BalusC
Copy link
Member Author

BalusC commented Mar 8, 2024 •
edited

It's controllable by output mode not by doctype. See also recently added IT on this, this is controlled via <facelets-processing> in faces-config.xml: https://github.com/jakartaee/faces/pull/1901/files#diff-04d1cac27952df9b1071dc16e7c90d9767fb24c48e6074a5ee82a31206d42ea7 The spec on this is here: https://jakarta.ee/specifications/faces/4.0/jakarta-faces-4.0#a7061 Note that the default value of <process-as> is thus html5 (since JSF 2.2).

Frankly I have no idea why exactly this is sone so, but the original impl did this since JSF 2.2. I believe this is because id attribute on head/body is unspecified/disallowed in HTML4 (even though XHTML1 and XML themselves allow "custom" attributes), but there is no dedicated output mode for HTML4 (obviously because it's not XML compatible).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
mojarra-implemented API issue implemented by Mojarra myfaces-implemented API issue implemented by MyFaces
Projects
None yet
Milestone
4.1
Development

No branches or pull requests
3 participants
@BalusC @tandraschko @volosied