OF-2549: Fix JavaScript error and improve UX in MUC Create Permission page#3239
OF-2549: Fix JavaScript error and improve UX in MUC Create Permission page#3239MilanTyagi2004 wants to merge 1 commit intoigniterealtime:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the admin console MUC room creation permissions page to eliminate a JavaScript focus error and make the “Allowed Users” section respond immediately to policy selection without requiring a reload.
Changes:
- Adds
toggleAllowedUsers()to show/hide the Allowed Users UI and conditionally focus the JID input. - Replaces server-side conditional rendering of the Allowed Users section with always-rendered markup that is toggled via CSS
display. - Removes broken inline click handlers that referenced the wrong form scope.
Comments suppressed due to low confidence (1)
xmppserver/src/main/webapp/muc-create-permission.jsp:252
- The visibility of
allowedUsersContaineris rendered frommucService.isRoomCreationRestricted(), but the new UI toggles it client-side based on the radio selection. If an admin selects “Only specific…” (shows the section) and then submits the Add/checkbox form, the server redirect can re-render withisRoomCreationRestricted()==false, hiding the section again even though the user just interacted with it. Consider persisting the selected policy across these submits (e.g., include/openPerms as a hidden field in the add form and/or derive initial display from a request param), or prevent using the Allowed Users form until the policy is saved.
<div id="allowedUsersContainer" style="display: <%= (mucService.isRoomCreationRestricted() ? "block" : "none") %>">
<!-- BEGIN 'Allowed Users' -->
<form action="muc-create-permission.jsp?add" method="post">
<input type="hidden" name="csrf" value="${csrf}">
<input type="hidden" name="mucname" value="<%= StringUtils.escapeForXML(mucname) %>" />
<div class="jive-contentBoxHeader">
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@guusdk changes are done, now this PR is okay to merge. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
xmppserver/src/main/webapp/muc-create-permission.jsp:252
- With the Allowed Users form always present in the DOM, it’s now possible to select “Only specific…” (without submitting the Save form) and submit the Add form. The server-side
addhandler does not enablesetRoomCreationRestricted(true), and after the redirect the page will revert to the current service setting (potentially hiding the Allowed Users section), which can make the Add appear to “disappear” and can leave configuration in an inconsistent/unclear state. Consider either preventing Add/Delete unless the service is restricted, or persisting/applying the policy choice when performing Add/Delete (eg submit the Save form automatically, or include/handle a policy param in the Add/Delete actions).
<div id="allowedUsersContainer" style="display: <%= (mucService.isRoomCreationRestricted() ? "block" : "none") %>">
<!-- BEGIN 'Allowed Users' -->
<form action="muc-create-permission.jsp?add" method="post">
<input type="hidden" name="csrf" value="${csrf}">
<input type="hidden" name="mucname" value="<%= StringUtils.escapeForXML(mucname) %>" />
<div class="jive-contentBoxHeader">
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @MilanTyagi2004 - I wonder if something is broken. When I open the page, and select "Only specific users/groups can create a chat room", and fill out a username, like below: If I then press the 'add' button, then I get a 'success' message, but the option "Anyone can create a chat room" is selected. 
You would expect the option "Only specific users/groups can create a chat room" to be selected! Can you please fix this? |
guusdk
left a comment
guusdk
left a comment
There was a problem hiding this comment.
(as mentioned in my previous commit: the setting no longer seems to be saved correctly).
|
sure i will do this and squash commits into one commit. |
i fix this, now it is working fine. |
|
hi @guusdk, |
- Fix issue where "Only specific users/groups" selection was not preserved after adding users by ensuring roomCreationRestricted is set correctly - Replace onclick with onchange on radio buttons to improve accessibility and ensure proper event handling - Remove auto-submit from "allow all registered users" checkbox to prevent unintended state resets - Improve UI responsiveness by correctly toggling Allowed Users section without page reload - Clean up JavaScript handling for safer focus behavior
|
I squashes all the commits into one with all description. |
|
@Fishbowler if you are not busy could you please review this. |
|
When I pick All Registered Users... 
...it's cleared on save 
|
Fishbowler
left a comment
Fishbowler
left a comment
There was a problem hiding this comment.
Note the issue above. I suspect this might be pre-existing, but I haven't checked.
Added one nitpick below.
| <meta name="helpPage" content="set_group_chat_room_creation_permissions.html"/> | ||
| <script> | ||
| function toggleAllowedUsers() { | ||
| const isRestricted = document.getElementById('rb02').checked; |
There was a problem hiding this comment.
If we're gonna use the ID in JS, would renaming the ID to something more friendly make sense?
There was a problem hiding this comment.
Thanks for review,
i will look into this.
This PR resolves a JavaScript error (Uncaught TypeError: Cannot read properties of undefined (reading 'focus')) on the MUC room creation permissions page and enhances the user interface to be more responsive.
Root Cause
The error was caused by a scope conflict where a radio button tried to focus an input field (userJID) located in a separate form. Additionally, the target field was often missing from the DOM because it was only rendered by the server when the room creation restriction was already enabled.
Changes
Dynamic Visibility: Replaced server-side JSP conditional rendering with client-side CSS toggling (display: block/none) for the "Allowed Users" section .
Robust Event Handling: Introduced a toggleAllowedUsers() JavaScript function to manage visibility and safely apply focus only when the element is available.
Improved UX: The "Allowed Users" section now appears or disappears immediately upon selecting a policy, without requiring a page reload.
Code Cleanup: Removed redundant and broken inline onclick handlers from JID and Group selection fields that were using incorrect this.form references.
Verification