resources/js/components/UserProfileImageSelector.vue (1)

112-119: Use template ref instead of document.getElementById.

Using document.getElementById is not idiomatic in Vue and can cause issues if multiple instances of this component exist simultaneously. Consider using a template ref instead.

♻️ Proposed fix using template ref

Update the template:

-      <img
-        :src="selectedFile"
-        width="100%"
-        id="cropper"
-        :alt="$t('admin.users.image.title')"
-      />
+      <img
+        ref="cropperImageRef"
+        :src="selectedFile"
+        width="100%"
+        :alt="$t('admin.users.image.title')"
+      />

Update the script:

 const cropper = ref();
+const cropperImageRef = ref(null);

-    cropper.value = new Cropper(document.getElementById("cropper"), {
+    cropper.value = new Cropper(cropperImageRef.value, {

Additionally, the static analysis tool indicates that id should go before width if you keep using the id attribute.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 112 - 119,
The template currently relies on an element id ("cropper") and likely uses
document.getElementById to access the image, which is not idiomatic and breaks
with multiple component instances; change the <img> to use a Vue template ref
(e.g., ref="cropperImg") and update any code that calls
document.getElementById("cropper") to use this.$refs.cropperImg (or the
Composition API ref) instead; also remove or keep the id attribute (preferably
remove it) or if kept follow static analysis ordering by placing id before width
in the <img> attributes. Ensure references to "selectedFile" remain unchanged
and update any methods that manipulate the cropper DOM to use the ref.

resources/js/components/UserProfileImageSelector.vue (2)

205-237: Consider adding error handling for robustness.

If file reading fails or the Cropper fails to initialize, isLoadingAction will remain true, leaving the UI in a stuck state. Consider adding error handling.

♻️ Suggested error handling pattern

 async function onFileSelect(event) {
   modalVisible.value = true;
   isLoadingAction.value = true;
   const file = event.files[0];

   const reader = new FileReader();
+  reader.onerror = () => {
+    isLoadingAction.value = false;
+    closeModal();
+    // Optionally show error toast/notification
+  };
   reader.onload = async (event) => {
     selectedFile.value = event.target.result;

     await nextTick();

-    cropper.value = new Cropper(document.getElementById("cropper"), {
+    try {
+      cropper.value = new Cropper(cropperImg.value, {
         // ... options
-    });
+      });
+    } catch (e) {
+      isLoadingAction.value = false;
+      closeModal();
+    }
   };
   reader.readAsDataURL(file);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 205 - 237,
The onFileSelect handler can leave isLoadingAction true if file reading or
Cropper initialization fails; add try/catch around the async work (FileReader
read and Cropper creation) so any thrown error sets isLoadingAction.value =
false and modalVisible.value = false (or closes the modal) and logs or surfaces
the error; specifically wrap the FileReader onload handler and the new
Cropper(...) creation/ready logic to catch initialization errors, ensure
selectedFile.value is only set on success, and in the catch block clean up
cropper.value if partially created and call
document.querySelector('[data-test="dialog-cancel-button"]').focus() (or
appropriate UI reset) to avoid a stuck loading state.

---

227-233: Consider using a template ref for focus management.

document.querySelector('[data-test="dialog-cancel-button"]') queries the entire document, which could select the wrong element if multiple instances exist. Consider adding a template ref to the cancel button for scoped access.

♻️ Proposed approach

Add a ref to the cancel button:

         <Button
+          ref="cancelButton"
           :label="$t('app.cancel')"
           severity="secondary"

In script:

+const cancelButton = ref(null);

Then focus using the ref:

-        document.querySelector('[data-test="dialog-cancel-button"]').focus();
+        cancelButton.value?.$el?.focus();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 227 - 233,
The ready method currently uses
document.querySelector('[data-test="dialog-cancel-button"]') which can collide
across instances; add a template ref (e.g., cancelButtonRef) to the cancel
button in the component template and replace the global query in the ready()
function with a scoped ref access: after await nextTick(), check
cancelButtonRef.value and call .focus() on it (with a null guard). Update any
related imports/`ref` declarations in the script setup so cancelButtonRef is
reactive and accessible in ready().

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

resources/js/components/UserProfileImageSelector.vue (1)

214-243: Handle FileReader error / cropper init failure.

If reader fails (or new Cropper(...) throws synchronously before ready fires), isLoadingAction stays true and the modal remains open with a spinner and no way to recover other than the cancel button. Add an onerror handler that closes the modal, and consider wrapping the Cropper construction in try/catch to reset loading state.

♻️ Proposed fix

   const reader = new FileReader();
   reader.onload = async (event) => {
     selectedFile.value = event.target.result;

     await nextTick();

-    cropper.value = new Cropper(cropperImgRef.value, {
+    try {
+      cropper.value = new Cropper(cropperImgRef.value, {
         aspectRatio: 1,
         autoCropArea: 0.9,
         background: false,
         guides: false,
         center: false,
         rotatable: false,
         zoomable: false,
         movable: false,
         viewMode: 1,
         dragMode: "none",
         ready: async function () {
           isLoadingAction.value = false;
         },
-    });
+      });
+    } catch (e) {
+      isLoadingAction.value = false;
+      closeModal();
+      throw e;
+    }
   };
+  reader.onerror = () => {
+    isLoadingAction.value = false;
+    closeModal();
+  };
   reader.readAsDataURL(file);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 214 - 243,
The onFileSelect handler currently sets modalVisible and isLoadingAction but
lacks error handling; add a reader.onerror callback that sets
isLoadingAction.value = false and modalVisible.value = false (or shows an error
state) and logs the error, and wrap the new Cropper(...) instantiation inside a
try/catch in onFileSelect (or inside reader.onload) so that if Cropper throws
synchronously you also reset isLoadingAction.value and modalVisible.value and
clean up any partially created cropper.value; reference reader, reader.onerror,
onFileSelect, cropper.value, Cropper, isLoadingAction.value and
modalVisible.value when implementing.

config/bigbluebutton.php (1)

13-13: Handle empty/whitespace env values when parsing BBB_ALLOWED_FILE_MIMES.

explode(',', '') yields [''] and tokens aren’t trimmed, so an empty or whitespace-padded override silently produces invalid entries that downstream File::types(...)/extensions(...) will then include in the allowed list.

♻️ Proposed fix

-    'allowed_file_mimes' => explode(',', env('BBB_ALLOWED_FILE_MIMES', 'pdf,doc,docx,xls,xlsx,ppt,pptx,txt,rtf,odt,ods,odp,odg,odc,odi,jpg,jpeg,png')),
+    'allowed_file_mimes' => array_values(array_filter(array_map('trim', explode(',', env('BBB_ALLOWED_FILE_MIMES', 'pdf,doc,docx,xls,xlsx,ppt,pptx,txt,rtf,odt,ods,odp,odg,odc,odi,jpg,jpeg,png'))), 'strlen')),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@config/bigbluebutton.php` at line 13, The config currently uses explode(',',
env('BBB_ALLOWED_FILE_MIMES', '...')) which yields [''] for empty env and
doesn't trim tokens; update the allowed_file_mimes parsing to split on commas,
trim each token, and filter out empty/whitespace-only entries (e.g., via
array_map('trim') + array_filter or preg_split with PREG_SPLIT_NO_EMPTY and
trim) so that allowed_file_mimes contains only valid non-empty extensions before
passing to File::types(...) / extensions(...).

resources/js/views/AdminSettings.vue (1)

1471-1477: Add a computed normalizer for bbb.file_mimes before passing to SettingsFileSelector.

While the backend always sends this setting as an array via explode() in config/bigbluebutton.php, the Visual test fixture contains a comma-separated string. Since SettingsFileSelector requires an array (uses .join() on the prop), add defensive normalization here—the only place in the codebase where this setting is dynamically bound to the component:

♻️ Proposed normalization

+                  :allowed-extensions="bbbFileMimes"
-                  :allowed-extensions="
-                    settingsStore.getSetting('bbb.file_mimes')
-                  "

Add near the other computed values:

const bbbFileMimes = computed(() => {
  const value = settingsStore.getSetting("bbb.file_mimes");

  if (Array.isArray(value)) {
    return value;
  }

  return String(value ?? "")
    .split(",")
    .map((extension) => extension.trim())
    .filter(Boolean);
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/views/AdminSettings.vue` around lines 1471 - 1477, Add a
defensive computed normalizer named bbbFileMimes that reads
settingsStore.getSetting('bbb.file_mimes') and returns an array (if already an
array return it; otherwise split the string on commas, trim each entry and
filter out empties), then replace the direct binding to
settingsStore.getSetting('bbb.file_mimes') passed into SettingsFileSelector with
the computed bbbFileMimes so the prop always receives an array compatible with
SettingsFileSelector (which uses .join()).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

resources/js/components/UserProfileImageSelector.vue (1)

15-15: ⚠️ Potential issue | 🔴 Critical

Resolve the still-present frontend lint/style failures.

Line 127 is a CI failure because useTemplateRef is unused; Lines 15, 104, and 119 still have the style warnings already reported.

🛠️ Proposed fix

-        :invalidFileSizeMessage="$t('app.validation.too_large')"
+        :invalid-file-size-message="$t('app.validation.too_large')"

           :disabled="isSavingAction"
           data-test="dialog-cancel-button"
-          `@click`="closeModal"
           autofocus
+          `@click`="closeModal"

       <img
         :src="selectedFile"
-        width="100%"
         ref="cropperImgRef"
+        width="100%"
         :alt="$t('admin.users.image.title')"

-import { nextTick, ref, useTemplateRef, watch } from "vue";
+import { nextTick, ref, watch } from "vue";

Also applies to: 103-104, 117-119, 127-127

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` at line 15, Remove or
use the unused template helper and any other unused bindings reported by the
linter: either delete the unused import/variable useTemplateRef from the
component setup or actually use it where intended, and similarly remove or use
any other unused variables/computeds flagged at the same locations; also fix the
style warnings around the prop binding
:invalidFileSizeMessage="$t('app.validation.too_large')" by aligning it with the
project formatting (consistent spacing/line breaks) so the component passes
frontend linting.

resources/js/components/UserProfileImageSelector.vue (1)

285-338: Scope handle lookup to this cropper instance.

document.getElementsByClassName(...)[0] can select another cropper’s handles or return undefined; the following setAttribute/addEventListener calls will then target the wrong DOM or throw.

♿ Safer scoped lookup pattern

 function initKeyboardShortcuts() {
   const dimensions = cropper.value.getData();
   scaleMove.value = Math.min(dimensions.width, dimensions.height) / 100;
+  const cropperRoot = cropper.value.container;
 
   // Get all handles
-  moveHandle.value = document.getElementsByClassName("cropper-move")[0];
-  topLeftHandle.value = document.getElementsByClassName(
-    "cropper-point point-nw",
-  )[0];
-  topRightHandle.value = document.getElementsByClassName(
-    "cropper-point point-ne",
-  )[0];
-  bottomLeftHandle.value = document.getElementsByClassName(
-    "cropper-point point-sw",
-  )[0];
-  bottomRightHandle.value = document.getElementsByClassName(
-    "cropper-point point-se",
-  )[0];
+  moveHandle.value = cropperRoot?.querySelector(".cropper-move");
+  topLeftHandle.value = cropperRoot?.querySelector(".cropper-point.point-nw");
+  topRightHandle.value = cropperRoot?.querySelector(".cropper-point.point-ne");
+  bottomLeftHandle.value = cropperRoot?.querySelector(".cropper-point.point-sw");
+  bottomRightHandle.value = cropperRoot?.querySelector(".cropper-point.point-se");
+
+  if (
+    !moveHandle.value ||
+    !topLeftHandle.value ||
+    !topRightHandle.value ||
+    !bottomLeftHandle.value ||
+    !bottomRightHandle.value
+  ) {
+    return;
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 285 - 338,
The element lookups in initKeyboardShortcuts currently use
document.getElementsByClassName(...) which can find handles from other cropper
instances or return undefined; change these to query the current cropper's
container (e.g., obtain the cropper DOM container from cropper.value via its API
or stored element, assign it to a local container variable) and call
container.getElementsByClassName(...) for moveHandle, topLeftHandle,
topRightHandle, bottomLeftHandle, bottomRightHandle; also guard each resulting
element (check for null/undefined) before calling setAttribute or
addEventListener so you never operate on the wrong or missing node.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

resources/js/components/UserProfileImageSelector.vue (1)

164-165: ⚠️ Potential issue | 🟠 Major

Make cropper initialization cancel-safe.

If the dialog is cancelled while FileReader, nextTick, or Cropper’s ready callback is still pending, stale callbacks can create a Cropper against a missing image ref or leave loading state stuck.

Guard stale async initialization

 const cropper = ref();
 const cropperImgRef = ref(null);
+const cropperInitToken = ref(0);
+const activeReader = ref(null);

 function closeModal() {
+  cropperInitToken.value += 1;
+  if (activeReader.value?.readyState === FileReader.LOADING) {
+    activeReader.value.abort();
+  }
+  activeReader.value = null;
+  isLoadingAction.value = false;
+
   if (cropper.value) {
     clearKeyboardShortcuts();
     cropper.value.destroy();
     cropper.value = null;
   }
@@
 async function onFileSelect(event) {
+  const initToken = ++cropperInitToken.value;
   modalVisible.value = true;
   isLoadingAction.value = true;
 
   const file = event.files[0];
 
   const reader = new FileReader();
-  reader.onload = async (event) => {
-    selectedFile.value = event.target.result;
+  activeReader.value = reader;
+  reader.onload = async (loadEvent) => {
+    if (initToken !== cropperInitToken.value || !modalVisible.value) {
+      return;
+    }
+
+    selectedFile.value = loadEvent.target.result;
 
     await nextTick();
 
+    if (
+      initToken !== cropperInitToken.value ||
+      !modalVisible.value ||
+      !cropperImgRef.value
+    ) {
+      isLoadingAction.value = false;
+      return;
+    }
+
     cropper.value = new Cropper(cropperImgRef.value, {
@@
       dragMode: "none",
       ready: async function () {
+        if (initToken !== cropperInitToken.value || !modalVisible.value) {
+          return;
+        }
+
         isLoadingAction.value = false;
 
         initKeyboardShortcuts();
       },
     });
   };
+  reader.onerror = reader.onabort = () => {
+    if (initToken === cropperInitToken.value) {
+      isLoadingAction.value = false;
+    }
+  };
   reader.readAsDataURL(file);
 }

Also applies to: 215-255

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 164 - 165,
The async Cropper initialization must be made cancel-safe: introduce a local
cancellation flag/token used in the image load flow that is set when the dialog
is closed/cancelled, abort any in-progress FileReader, and return early from
pending callbacks; before calling new Cropper(...) or accessing
cropperImgRef.value validate the ref and the token, and in Cropper's ready
callback check the token before touching state; also ensure cropper (ref) is
destroyed and loading state cleared on cancel/cleanup (affects the functions
using cropper, cropperImgRef, FileReader, nextTick, and Cropper.ready in the
component).

resources/js/components/UserProfileImageSelector.vue (1)

299-311: Nit: pass the class name as a single token, not a compound selector string.

document.getElementsByClassName("cropper-point point-nw") happens to work (the argument is interpreted as a space-separated class list — elements must have both classes), but it's easy to misread as a CSS selector. Using querySelector(".cropper-point.point-nw") scoped to the cropper's container is clearer and less coupled to global DOM state.

-  moveHandle.value = document.getElementsByClassName("cropper-move")[0];
-  topLeftHandle.value = document.getElementsByClassName(
-    "cropper-point point-nw",
-  )[0];
+  const root = cropperImgRef.value?.closest(".cropper-container") ?? document;
+  moveHandle.value = root.querySelector(".cropper-move");
+  topLeftHandle.value = root.querySelector(".cropper-point.point-nw");
   // …etc

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 299 - 311,
The code is using document.getElementsByClassName with compound class strings
(e.g., "cropper-point point-nw") and global document access; change these to
select elements more clearly and scoped to the cropper container: either call
getElementsByClassName with a single token (e.g., "cropper-point") and filter
for the second class, or better, use
container.querySelector(".cropper-point.point-nw") (or
container.querySelector(".cropper-move") for moveHandle) so you avoid global DOM
coupling; update where moveHandle, topLeftHandle, topRightHandle,
bottomLeftHandle, and bottomRightHandle are assigned to use the scoped
querySelector approach.

tests/Frontend/support/utils/fileHelper.js (2)

43-56: Add onerror handling to avoid test hangs on decode failures.

_base64ToImageData only resolves from image.onload. If the base64 input is malformed or decoding fails, the promise never settles and tests will hang until Cypress/Mocha times out, with no useful diagnostic. Also, no reject path is plumbed through the returned promise.

♻️ Proposed fix

 export async function _base64ToImageData(base64) {
-  return await new Promise((res) => {
+  return await new Promise((resolve, reject) => {
     const image = new Image();
     image.src = base64;

     image.onload = () => {
       const { naturalWidth: width, naturalHeight: height } = image;
       const canvas = new OffscreenCanvas(width, height);
       const ctx = canvas.getContext("2d");
       ctx.drawImage(image, 0, 0);
-      res(ctx.getImageData(0, 0, width, height));
+      resolve(ctx.getImageData(0, 0, width, height));
     };
+    image.onerror = () =>
+      reject(new Error("Failed to decode base64 image for comparison"));
   });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/Frontend/support/utils/fileHelper.js` around lines 43 - 56,
_base64ToImageData currently only resolves on image.onload so malformed base64
will leave the promise pending; update the Promise executor to accept (res, rej)
and add an image.onerror handler that calls rej(new Error(...)) with a
descriptive message (include the failing base64 or its prefix), and also guard
for ctx being null before calling getImageData and reject if the 2D context is
unavailable; ensure both onload and onerror clean up handlers if needed so the
promise always settles.

---

23-40: Minor cleanups in _compareBase64Images.

• The variable is named diff2 (likely a leftover from iteration); rename to diff for clarity.
• The failure message embeds the full base64 strings, which for typical profile images can be many KB and clutter the test output. Consider omitting the data URIs (or truncating them) and instead logging the diff pixel count.

♻️ Proposed fix

-  const diff2 = pixelmatch(
+  const diff = pixelmatch(
     expectedImage.data,
     actualImage.data,
     null,
     expectedImage.width,
     expectedImage.height,
     {
       threshold: 0.1,
     },
   );

-  expect(diff2).to.eql(
-    0,
-    "Expected Image: " +
-      expectedImageBase64 +
-      " Actual Image: " +
-      actualImageBase64,
-  );
+  expect(diff).to.eql(0, `Images differ by ${diff} pixels`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/Frontend/support/utils/fileHelper.js` around lines 23 - 40, In
_compareBase64Images, rename the local variable diff2 to diff for clarity and
update the assertion message to avoid embedding full base64 blobs; instead
include the diff pixel count and a short preview (e.g., first 64 chars) of
expectedImageBase64 and actualImageBase64 or omit them entirely. Change the
expect call on pixelmatch's result (now diff) to report something like "Image
mismatch: X differing pixels. Expected (prefix): <...> Actual (prefix): <...>"
so test output is concise and useful.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes