JSF: avoid suggesting UIComponent default attributes on non-component tags - #9606
NicolaIsotta wants to merge 1 commit into
Conversation
… tags TagImpl no longer inherits generic UIComponent attributes (like 'rendered') across all facelet tags. Introduce a ComponentTag wrapper so default component attributes are only provided for actual UI/composite components. Add tests for the corrections, even if CI doesn't run them at the moment
|
To give a visual example, these are the attributes suggestions of For reference: https://jakarta.ee/specifications/faces/4.1/vdldoc/f/ajax |
mbien
added
Java EE/Jakarta EE
|
BTW, I managed to make the web.jsf.editor unit tests work. Can I add another commit to this PR, or is it better to open a new one? |
Please open a new PR for that unless the changes depend on each other. Thank you! |
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
I think this makes sense to me. I left an inline comment and a clarification request to help my understanding. Thank you!
| Collection<Attribute> all = new ArrayList<>(super.getAttributes()); | ||
| all.addAll(attrs.values()); | ||
| return all; | ||
| return attrs.values(); |
There was a problem hiding this comment.
I suggest to wrap the Map passed into the constructor into Collections#unmodifiableMap- before the change getAttributes returned a copy and thus changes could not propagate, that is now different.
|
|
||
| // TagHandlers must NOT have UIComponent default attributes | ||
| assertNull(t2.getAttribute("rendered")); | ||
| assertNull(t2.getAttribute("transient")); |
There was a problem hiding this comment.
For my understanding. I see this declaration:
This implies that HyperComponent is in fact a UIComponent. Does this not become relevant because the tag library refers to HyperComponent only via handler-class and not via component-type?
There was a problem hiding this comment.
exactly, the taglib definition is the "source of truth" in this case
created #9617 - it might be worth merging that (and enabling web.jsf.editor tests in CI) before this |
3 participants
TagImpl no longer inherits generic UIComponent attributes (like 'rendered') across all facelet tags. Introduce a ComponentTag wrapper so default component attributes are only provided for actual UI/composite components.
Add tests for the corrections, even if CI doesn't run them at the moment
^Add meaningful description above
Click to collapse/expand PR instructions
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
LLMs, Commit messages and PR description:
git log) that all commits have a valid name and email address for you in the Author field.Assisted-by: MODEL_NAME MODEL_VERSIONline appended to the commit message.Assisted-byline from above)If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)