-
Notifications
You must be signed in to change notification settings - Fork 16
feat/CUS-10170-Added support for the latest chrome permissions #306
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
base: dev
Are you sure you want to change the base?
feat/CUS-10170-Added support for the latest chrome permissions #306
Conversation
📝 WalkthroughWalkthroughThe pull request bumps the artifact version to 1.0.8 and updates the TestSigma SDK dependency to 1.2.24_cloud. It introduces runtime detection logic for different Chrome settings DOM structures (Chrome 143+ versus older versions) in two action classes, adding conditional branching to select appropriate DOM paths based on detected structure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@chrome_site_permissions/src/main/java/com/testsigma/addons/mobileweb/MyFirstWebAction.java`:
- Around line 55-83: The JS DOM probes assigned to
isOldStructure/isChrome143Structure and the permissionSelect string builders in
MyFirstWebAction.java can throw TypeErrors when intermediate shadowRoots are
null/closed; modify the injected scripts used in the js.executeScript calls and
the permissionSelect template strings (the snippets referenced by
isOldStructure, isChrome143Structure and the permissionSelect assignment
branches) to use optional chaining at every step (use ?. between each
.shadowRoot and .querySelector call) so each probe safely returns null/undefined
instead of throwing, and ensure the final selector access also uses optional
chaining before querying `#permission`.
In
`@chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java`:
- Around line 55-83: The DOM-probing scripts used to set isOldStructure and
isChrome143Structure can throw TypeError when a shadowRoot is null because
optional chaining is only applied to the first hop; update the JS strings passed
to js.executeScript (the expressions assigned to isOldStructure and
isChrome143Structure and any similar probes) to apply optional chaining before
every .querySelector call after accessing shadowRoot (e.g., use
?.querySelector(...) at each step) so each step returns undefined instead of
throwing, and ensure the final expression yields a Boolean (use !!(...) or
explicit Boolean(...) at the end); keep the same variables (isOldStructure,
isChrome143Structure, permissionSelect) and logging calls but replace the unsafe
script snippets with the fully safe optional-chained versions.
| // Determine which permission selector to use | ||
| Boolean isOldStructure = (Boolean) js.executeScript( | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');" | ||
| ); | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');"); | ||
|
|
||
| logger.info("Is old structure flag: " + isOldStructure); | ||
| Boolean isChrome143Structure = (Boolean) js.executeScript( | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-privacy-page-index')?.shadowRoot.querySelector('#siteSettingsSiteDetails');"); | ||
|
|
||
| logger.info("Is Chrome 143+ structure: " + isChrome143Structure); | ||
| String permissionSelect; | ||
| if (isOldStructure) { | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" + | ||
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" + | ||
|
|
||
| if (isChrome143Structure) { | ||
| logger.info("Its Chrome 143+ Structure"); | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\")\n" + | ||
| " .shadowRoot.querySelector(\"#main\")\n" + | ||
| " .shadowRoot.querySelector(\"settings-privacy-page-index\")\n" + | ||
| " .shadowRoot.querySelector(\"#siteSettingsSiteDetails\")\n" + | ||
| " .shadowRoot.querySelector(\"site-details-permission[label='%s']\")\n" + | ||
| " .shadowRoot.querySelector(\"#permission\")"; | ||
| } else if (isOldStructure) { | ||
| logger.info("Its Old Structure"); | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" | ||
| + | ||
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" | ||
| + | ||
| ".querySelector(\"#pages > settings-subpage > site-details\").shadowRoot" + | ||
| ".querySelector(\"div.list-frame > site-details-permission[label='%s']\").shadowRoot.querySelector(\"#permission\")"; | ||
| } else { | ||
| logger.info("Its New Structure (pre-143)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
JavaScript optional chaining shadowRoot querySelector TypeError
💡 Result:
Most likely causes
- You put the optional-chaining operator in the wrong place (so you still access a property on null). Example mistake: elem.shadowRoot.querySelector?.('.foo') — this tries to read querySelector from shadowRoot (throws if shadowRoot is null) then uses optional-call. Use the guard on shadowRoot instead. [1][2]
- shadowRoot is actually null because the element isn’t attached / not yet rendered or its shadow DOM is closed. Querying too early (or for a closed shadow root) yields null and then querySelector on that null throws. [3]
- In TypeScript the host variable may be typed as Node (which lacks querySelector) so you get type errors rather than runtime ones. Cast/declare correct type. [4]
Fixes (concise)
- Correct optional-chaining placement:
- Good: const node = elem.shadowRoot?.querySelector('.selector'); // safe: returns undefined if no shadowRoot. [1][2]
- Bad: const node = elem.shadowRoot.querySelector?.('.selector'); // will throw if shadowRoot is null. [1]
- Ensure element is ready (e.g., run after connectedCallback / DOMContentLoaded / requestAnimationFrame) or check existence:
- if (elem.shadowRoot) elem.shadowRoot.querySelector('.sel')... or elem.shadowRoot?.querySelector('.sel')
- If using methods and they may be missing, use optional-call on the function reference (only after safely accessing it):
- const fn = elem.shadowRoot?.someMethod; fn?.();
- TypeScript: narrow/cast to Element / ShadowRoot: const sr = (elem as HTMLElement).shadowRoot; sr?.querySelector(...). [4]
Browser/support note
- Optional chaining is ES2020 — available in modern browsers; transpile for older targets. [1]
References
- Optional chaining usage and gotchas. [1]
- Examples for shadowRoot access and common timing issues. [3]
- TypeScript: querySelector vs Node/Element typing. [4]
Sources
[1] Dev article & optional-chaining examples. (see discussion of property vs call placement). (allthingssmitty.com)
[2] Shadow-root query examples and correct optional-chaining usage in real code. (sitepoint.com)
[3] ShadowRoot may be null / timing and polyfill differences. (stackoverflow.com)
[4] TypeScript: Property 'querySelector' does not exist on type 'Node' — cast/declare fixes. (stackoverflow.com)
If you paste the exact line that throws the TypeError I’ll point out the precise fix.
Citations:
- 1: https://allthingssmitty.com/2025/06/02/write-more-reliable-javascript-with-optional-chaining/?utm_source=openai
- 2: https://www.sitepoint.com/community/t/how-to-select-an-element-in-a-shadow-root/430046?utm_source=openai
- 3: https://stackoverflow.com/questions/47233481/attached-shadowroot-using-polyfill-is-not-query-able?utm_source=openai
- 4: https://stackoverflow.com/questions/31237237/property-queryselector-does-not-exist-on-type-node-in-typescript?utm_source=openai
🌐 Web query:
JavaScript optional chaining operator . vs ?. behavior specification
💡 Result:
Summary — how . (dot) vs ?. (optional chaining) behave (spec):
-
. (normal property access/call)
- Evaluates the base; if the base value is null or undefined, attempting to access a property or call will throw a TypeError. Use in LHS assignment, new, tagged templates, etc., is allowed per normal semantics. [1]
-
?. (optional chaining)
- Short-circuits: if the base value is null or undefined, the whole optional-chain expression evaluates to undefined instead of throwing. It applies to property access (obj?.prop, obj?.[expr]) and calls (fn?.(...)). Subsequent accesses in the same continuous chain are not evaluated once short-circuited. [2][1]
- It does not create a temporary variable in user code; the spec defines precise evaluation through ChainEvaluation/OptionalExpression semantics (GetValue, short-circuit, then evaluate the remaining chain). This also preserves proper this-binding when used for method calls (the spec uses EvaluateCall/EvaluatePropertyAccess semantics). [1]
- Some uses are forbidden or invalid: optional chains cannot be the left-hand side of assignment (object?.prop = … is a SyntaxError), cannot be used directly with new (new F?.()), and cannot form tagged template targets. Grouping can break the chain (e.g., (a?.b).c will attempt .c on undefined). [2][1]
References
- ECMAScript Language Specification — Optional Chains / ChainEvaluation (runtime semantics). [1]
- MDN Web Docs — Optional chaining (?.) operator (examples, restrictions). [2]
- TC39 proposal text for Optional Chaining (normative background and syntax). [3]
🏁 Script executed:
# Check if there are test cases or other examples in the codebase that might show similar patterns
git ls-files | grep -E '\.(java|js)$' | head -20Repository: testsigmahq/testsigma-addons
Length of output: 200
🌐 Web query:
optional chaining null propagation JavaScript optional chaining not working with chained property access
💡 Result:
Short answer: optional chaining only protects the immediate access it’s attached to. Use ?. at each step (or avoid grouping) so you don’t evaluate a property on an undefined/null value. See MDN for details. [1]
Examples:
- Safe when root may be nullish:
const x = obj?.a.b; // safe if obj is null/undefined — returns undefined - Unsafe if an intermediate may be nullish:
const y = (obj?.a).b; // throws if obj?.a is undefined because .b is evaluated - To protect every step, chain the operator:
const z = obj?.a?.b?.c; // safe even if a or b are null/undefined
Notes: optional chaining short-circuits only the expression immediately to its right and cannot be used on the left side of an assignment. [1]
Source: MDN — Optional chaining (?.). [1]
Harden DOM-structure probes to avoid JS TypeErrors on missing or closed shadow roots.
The optional chaining only guards the immediate operation it's attached to; if a shadowRoot is null or closed, .querySelector(...) can still throw. Chain ?. through each shadowRoot and querySelector to ensure probes reliably return undefined/null instead of erroring.
🔧 Correct optional chaining placement
- Boolean isOldStructure = (Boolean) js.executeScript(
- "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');");
+ Boolean isOldStructure = (Boolean) js.executeScript(
+ "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-basic-page');");
...
- Boolean isChrome143Structure = (Boolean) js.executeScript(
- "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-privacy-page-index')?.shadowRoot.querySelector('#siteSettingsSiteDetails');");
+ Boolean isChrome143Structure = (Boolean) js.executeScript(
+ "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-privacy-page-index')?.shadowRoot?.querySelector('#siteSettingsSiteDetails');");🤖 Prompt for AI Agents
In
`@chrome_site_permissions/src/main/java/com/testsigma/addons/mobileweb/MyFirstWebAction.java`
around lines 55 - 83, The JS DOM probes assigned to
isOldStructure/isChrome143Structure and the permissionSelect string builders in
MyFirstWebAction.java can throw TypeErrors when intermediate shadowRoots are
null/closed; modify the injected scripts used in the js.executeScript calls and
the permissionSelect template strings (the snippets referenced by
isOldStructure, isChrome143Structure and the permissionSelect assignment
branches) to use optional chaining at every step (use ?. between each
.shadowRoot and .querySelector call) so each probe safely returns null/undefined
instead of throwing, and ensure the final selector access also uses optional
chaining before querying `#permission`.
| // Determine which permission selector to use | ||
| Boolean isOldStructure = (Boolean) js.executeScript( | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');" | ||
| ); | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');"); | ||
|
|
||
| logger.info("Is old structure flag: " + isOldStructure); | ||
| Boolean isChrome143Structure = (Boolean) js.executeScript( | ||
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-privacy-page-index')?.shadowRoot.querySelector('#siteSettingsSiteDetails');"); | ||
|
|
||
| logger.info("Is Chrome 143+ structure: " + isChrome143Structure); | ||
| String permissionSelect; | ||
| if (isOldStructure) { | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" + | ||
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" + | ||
|
|
||
| if (isChrome143Structure) { | ||
| logger.info("Its Chrome 143+ Structure"); | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\")\n" + | ||
| " .shadowRoot.querySelector(\"#main\")\n" + | ||
| " .shadowRoot.querySelector(\"settings-privacy-page-index\")\n" + | ||
| " .shadowRoot.querySelector(\"#siteSettingsSiteDetails\")\n" + | ||
| " .shadowRoot.querySelector(\"site-details-permission[label='%s']\")\n" + | ||
| " .shadowRoot.querySelector(\"#permission\")"; | ||
| } else if (isOldStructure) { | ||
| logger.info("Its Old Structure"); | ||
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" | ||
| + | ||
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" | ||
| + | ||
| ".querySelector(\"#pages > settings-subpage > site-details\").shadowRoot" + | ||
| ".querySelector(\"div.list-frame > site-details-permission[label='%s']\").shadowRoot.querySelector(\"#permission\")"; | ||
| } else { | ||
| logger.info("Its New Structure (pre-143)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java | head -n 100Repository: testsigmahq/testsigma-addons
Length of output: 200
🌐 Web query:
JavaScript optional chaining shadowRoot querySelector TypeError null behavior
💡 Result:
Short answer: optional chaining (?.) only protects the single access/call it's placed on — it returns undefined if the left operand is null or undefined. It does NOT magically protect later property accesses on the result. In the shadowRoot + querySelector case you must account for two separate nullish possibilities: the shadowRoot itself may be null, and querySelector may return null when no element matches. Use optional chaining (and/or nullish coalescing) at each step.
Examples:
-
Protect shadowRoot itself:
const el = host.shadowRoot?.querySelector('.foo');
// If host.shadowRoot is null/undefined, el === undefined (no TypeError). -
Protect querySelector’s result before reading a property:
const text = host.shadowRoot?.querySelector('.foo')?.textContent;
// If shadowRoot is null → overall result undefined. If querySelector returns null → ?. prevents TypeError and returns undefined.
// If you instead wrote (host.shadowRoot?.querySelector('.foo')).textContent you can still get a TypeError because the optional chain ended before .textContent was accessed. (developer.mozilla.org)
Notes:
- querySelector returns null when no match; optional chaining on the querySelector call prevents calling a method on null but does not change querySelector’s null return value — so continue to use ?., &&, or ?? when accessing properties of the matched element. (developer.mozilla.org)
Sources:
[1] MDN — Optional chaining operator (?.).
[2] MDN — Optional chaining (examples and short-circuiting).
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining.?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining.?utm_source=openai
Harden DOM-structure probes to avoid JS TypeErrors on missing shadow roots.
The optional chaining only guards the first hop; if a shadowRoot is null/closed, .querySelector(...) can still throw and the action fails before any fallback. The ?. operator only protects the single operation it's placed on—it must be applied before each .querySelector() call following a shadowRoot access to reliably return undefined instead of throwing a TypeError.
🔧 Suggested safe optional chaining
- Boolean isOldStructure = (Boolean) js.executeScript(
- "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');");
+ Boolean isOldStructure = (Boolean) js.executeScript(
+ "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-basic-page');");
...
- Boolean isChrome143Structure = (Boolean) js.executeScript(
- "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-privacy-page-index')?.shadowRoot.querySelector('#siteSettingsSiteDetails');");
+ Boolean isChrome143Structure = (Boolean) js.executeScript(
+ "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-privacy-page-index')?.shadowRoot?.querySelector('#siteSettingsSiteDetails');");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine which permission selector to use | |
| Boolean isOldStructure = (Boolean) js.executeScript( | |
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');" | |
| ); | |
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-basic-page');"); | |
| logger.info("Is old structure flag: " + isOldStructure); | |
| Boolean isChrome143Structure = (Boolean) js.executeScript( | |
| "return !!document.querySelector('body > settings-ui')?.shadowRoot.querySelector('#main')?.shadowRoot.querySelector('settings-privacy-page-index')?.shadowRoot.querySelector('#siteSettingsSiteDetails');"); | |
| logger.info("Is Chrome 143+ structure: " + isChrome143Structure); | |
| String permissionSelect; | |
| if (isOldStructure) { | |
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" + | |
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" + | |
| if (isChrome143Structure) { | |
| logger.info("Its Chrome 143+ Structure"); | |
| permissionSelect = "return document.querySelector(\"body > settings-ui\")\n" + | |
| " .shadowRoot.querySelector(\"#main\")\n" + | |
| " .shadowRoot.querySelector(\"settings-privacy-page-index\")\n" + | |
| " .shadowRoot.querySelector(\"#siteSettingsSiteDetails\")\n" + | |
| " .shadowRoot.querySelector(\"site-details-permission[label='%s']\")\n" + | |
| " .shadowRoot.querySelector(\"#permission\")"; | |
| } else if (isOldStructure) { | |
| logger.info("Its Old Structure"); | |
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" | |
| + | |
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" | |
| + | |
| ".querySelector(\"#pages > settings-subpage > site-details\").shadowRoot" + | |
| ".querySelector(\"div.list-frame > site-details-permission[label='%s']\").shadowRoot.querySelector(\"#permission\")"; | |
| } else { | |
| logger.info("Its New Structure (pre-143)"); | |
| // Determine which permission selector to use | |
| Boolean isOldStructure = (Boolean) js.executeScript( | |
| "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-basic-page');"); | |
| logger.info("Is old structure flag: " + isOldStructure); | |
| Boolean isChrome143Structure = (Boolean) js.executeScript( | |
| "return !!document.querySelector('body > settings-ui')?.shadowRoot?.querySelector('#main')?.shadowRoot?.querySelector('settings-privacy-page-index')?.shadowRoot?.querySelector('#siteSettingsSiteDetails');"); | |
| logger.info("Is Chrome 143+ structure: " + isChrome143Structure); | |
| String permissionSelect; | |
| if (isChrome143Structure) { | |
| logger.info("Its Chrome 143+ Structure"); | |
| permissionSelect = "return document.querySelector(\"body > settings-ui\")\n" + | |
| " .shadowRoot.querySelector(\"#main\")\n" + | |
| " .shadowRoot.querySelector(\"settings-privacy-page-index\")\n" + | |
| " .shadowRoot.querySelector(\"#siteSettingsSiteDetails\")\n" + | |
| " .shadowRoot.querySelector(\"site-details-permission[label='%s']\")\n" + | |
| " .shadowRoot.querySelector(\"#permission\")"; | |
| } else if (isOldStructure) { | |
| logger.info("Its Old Structure"); | |
| permissionSelect = "return document.querySelector(\"body > settings-ui\").shadowRoot.querySelector(\"#main\").shadowRoot" | |
| ".querySelector(\"settings-basic-page\").shadowRoot.querySelector(\"#basicPage > settings-section.expanded > settings-privacy-page\").shadowRoot" | |
| ".querySelector(\"#pages > settings-subpage > site-details\").shadowRoot" + | |
| ".querySelector(\"div.list-frame > site-details-permission[label='%s']\").shadowRoot.querySelector(\"#permission\")"; | |
| } else { | |
| logger.info("Its New Structure (pre-143)"); |
🤖 Prompt for AI Agents
In
`@chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java`
around lines 55 - 83, The DOM-probing scripts used to set isOldStructure and
isChrome143Structure can throw TypeError when a shadowRoot is null because
optional chaining is only applied to the first hop; update the JS strings passed
to js.executeScript (the expressions assigned to isOldStructure and
isChrome143Structure and any similar probes) to apply optional chaining before
every .querySelector call after accessing shadowRoot (e.g., use
?.querySelector(...) at each step) so each step returns undefined instead of
throwing, and ensure the final expression yields a Boolean (use !!(...) or
explicit Boolean(...) at the end); keep the same variables (isOldStructure,
isChrome143Structure, permissionSelect) and logging calls but replace the unsafe
script snippets with the fully safe optional-chained versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
chrome_site_permissions/pom.xml (1)
26-31: Duplicatecommons-lang3dependency with conflicting versions.The dependency is declared twice: version 3.13.0 on lines 27-31 and version 3.17.0 on lines 72-75. Maven will use one arbitrarily, causing unpredictable behavior and build warnings.
🔧 Proposed fix: Remove the duplicate
- <dependency> - <groupId>org.apache.commons</groupId> - <artifactId>commons-lang3</artifactId> - <version>3.13.0</version> - <scope>compile</scope> - </dependency>Keep only the 3.17.0 version (lines 72-75) as it's newer.
Also applies to: 72-75
chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java (1)
46-50: UseorElseThrow()instead of bare.get()for safer Optional handling.If no new tab handle is found,
.get()throws a genericNoSuchElementExceptionwithout context. UsingorElseThrow()provides a clearer error message.🔧 Proposed fix
String newTab = driver.getWindowHandles() .stream() .filter(handle -> !origWindowsList.contains(handle)) .findFirst() - .get(); + .orElseThrow(() -> new IllegalStateException("Failed to open new tab for Chrome settings"));chrome_site_permissions/src/main/java/com/testsigma/addons/mobileweb/MyFirstWebAction.java (1)
46-50: UseorElseThrow()for clearer error handling.Same issue as web variant—bare
.get()on Optional provides no context if no new tab is found.🔧 Proposed fix
String newTab = driver.getWindowHandles() .stream() .filter(handle -> !origWindowsList.contains(handle)) .findFirst() - .get(); + .orElseThrow(() -> new IllegalStateException("Failed to open new tab for Chrome settings"));
♻️ Duplicate comments (2)
chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java (1)
56-61: Incomplete optional chaining in DOM probes.The JS probes use
?.shadowRoot.querySelectorbut should use?.shadowRoot?.querySelectorat each step to avoid TypeErrors when intermediate shadowRoots are null.chrome_site_permissions/src/main/java/com/testsigma/addons/mobileweb/MyFirstWebAction.java (1)
56-61: Incomplete optional chaining in DOM probes.Same issue as the web variant—use
?.shadowRoot?.querySelectorat each step to prevent TypeErrors on null shadowRoots.
🧹 Nitpick comments (2)
chrome_site_permissions/src/main/java/com/testsigma/addons/web/MyFirstWebAction.java (1)
53-53: Consider replacingThread.sleepwith explicit waits.Hardcoded sleeps (5000ms, 3000ms) are brittle—they may be too short on slow systems or wastefully long on fast ones. An explicit wait for DOM readiness would be more reliable.
Also applies to: 104-104
chrome_site_permissions/src/main/java/com/testsigma/addons/mobileweb/MyFirstWebAction.java (1)
31-119: Consider extracting shared logic to reduce duplication.The
execute()method in bothweb/MyFirstWebAction.javaandmobileweb/MyFirstWebAction.javais nearly identical—onlyApplicationTypediffers. Extracting common logic into a shared base class or utility would reduce maintenance burden when Chrome DOM structures change again.
Publish this addon as public
Addon Name: Chrome site permissions
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-10170
Added support for the latest chrome permissions
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.