D49300_Svg Child element Support Added#4294
Merged
Merged
Conversation
Contributor
WalkthroughAdds comprehensive SVG discovery, locator/property generation, and interaction handling in the Selenium driver; introduces ElementInfo LearnedType (Manual/Auto/AI) and surfaces it in POM UI; enables Add/Delete and scrolling for the Properties grid; adds a UIElementFilter flag; and unconditionally binds the AI learn toggle in the POM wizard. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant POM as "POM Learner"
participant SD as "SeleniumDriver"
participant B as Browser
User->>POM: Start POM Learn
POM->>SD: Request element discovery
SD->>B: Query DOM / spy
B-->>SD: WebElements / node info
alt element is SVG
SD->>SD: IsSvgElement -> Generate enhanced SVG XPath
SD->>SD: ProcessSvgChildElements -> add SVG locators/properties, set ParentSVGElement
SD->>POM: Return parent + child SVG ElementInfo (LearnedType)
else non-SVG element
SD->>SD: Standard element processing
SD->>POM: Return HTMLElementInfo
end
POM-->>User: Display learned elements (with LearnedType)
sequenceDiagram
autonumber
actor User
participant Runner
participant SD as "SeleniumDriver"
participant B as Browser
User->>Runner: Trigger UI action (Click/DoubleClick/JS)
Runner->>SD: ActUIElementHandler(action)
SD->>SD: DoUIElementClick(selects strategy)
SD->>B: Execute click strategy
B-->>SD: Result / Exception
SD-->>Runner: Status (Success / Failed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (3)
219-229: Keep AI toggle visible on Web; disable when not configured/agent not running.For discoverability, set Visibility to Visible on Web regardless of config and rely on IsEnabled (already controlled in UpdateConfigsBasedOnAgentStatus). Hide only for non-Web.
- if(GingerPlayConfiguration.IsGingerPlayConfigured()) - { - xLearnPOMByAI.Visibility = Visibility.Visible; - } + xLearnPOMByAI.Visibility = Visibility.Visible; ... - xLearnPOMByAI.Visibility = Visibility.Collapsed; + xLearnPOMByAI.Visibility = Visibility.Collapsed // keep hidden for non-Web
117-150: Prevent duplicate event subscriptions and potential leaks.SubscribeToCurrentSeleniumDriver can be called multiple times (on agent run/selection). Guard against duplicate subscriptions; optionally track and unsubscribe on page exit.
private void SubscribeToCurrentSeleniumDriver() { try { @@ - if (currentAgent?.AgentOperations is AgentOperations agentOps && agentOps.Driver is SeleniumDriver seleniumDriver) + if (currentAgent?.AgentOperations is AgentOperations agentOps && agentOps.Driver is SeleniumDriver seleniumDriver) { // Get the wizard window and subscribe to driver events if (mBasePOMWizard?.mWizardWindow is WizardWindow wizardWindow) { - wizardWindow.SubscribeToSeleniumDriver(seleniumDriver); - Reporter.ToLog(eLogLevel.DEBUG, "Successfully subscribed to SeleniumDriver AI processing events"); + if (!ReferenceEquals(_subscribedSeleniumDriver, seleniumDriver)) + { + // Idempotent subscribe + wizardWindow.SubscribeToSeleniumDriver(seleniumDriver); + _subscribedSeleniumDriver = seleniumDriver; + Reporter.ToLog(eLogLevel.DEBUG, "Successfully subscribed to SeleniumDriver AI processing events"); + } } }Add this field in the class:
private SeleniumDriver _subscribedSeleniumDriver;Optionally, on EventType.LeavingForNextPage, clear the reference (and unsubscribe if an API exists).
111-114: Clean up on page exit.Clear the tracked subscription on navigation to avoid stale references; unsubscribe if API exists.
case EventType.LeavingForNextPage: UpdateCustomTemplateList(); + _subscribedSeleniumDriver = null; // If Unsubscribe API exists, call it here. break;Please confirm whether WizardWindow exposes UnsubscribeFromSeleniumDriver; if yes, we should call it here.
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WebPlatformInfo.cs (1)
451-462: Add default operation for Svg to Click.Svg elements now support click-like actions; GetDefaultElementOperation should return Click for Svg to avoid defaulting to NotExist.
public override string GetDefaultElementOperation(eElementType ElementTypeEnum) { return ElementTypeEnum switch { - eElementType.Button or eElementType.CheckBox or eElementType.RadioButton or eElementType.HyperLink or eElementType.Span or eElementType.Div or eElementType.Image => ActUIElement.eElementAction.Click.ToString(),//actConfig.Operation = ActUIElement.eElementAction.Click.ToString(); + eElementType.Button or eElementType.CheckBox or eElementType.RadioButton or eElementType.HyperLink or eElementType.Span or eElementType.Div or eElementType.Image or eElementType.Svg + => ActUIElement.eElementAction.Click.ToString(), eElementType.TextBox => ActUIElement.eElementAction.SetText.ToString(),//actConfig.Operation = ActUIElement.eElementAction.SetText.ToString(); eElementType.Iframe => ActBrowserElement.eControlAction.SwitchFrame.ToString(),//actConfig.Operation = ActUIElement.eElementAction.SetText.ToString(); eElementType.ComboBox => ActUIElement.eElementAction.SelectByText.ToString(), eElementType.Label or eElementType.Text => ActUIElement.eElementAction.GetValue.ToString(), _ => ActUIElement.eElementAction.NotExist.ToString(),//actConfig.Operation = ActUIElement.eElementAction.NotExist.ToString(); }; }Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (1)
536-541: Optional: Extend edit lock to AI-learned elements.Currently only auto-learned items are non-editable. Consider also locking when LearnedType is AI to keep behavior consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs(1 hunks)Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs(1 hunks)Ginger/GingerCoreCommon/UIElement/ElementInfo.cs(4 hunks)Ginger/GingerCoreCommon/UIElement/UIElementFilter.cs(1 hunks)Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs(3 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(12 hunks)Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WebPlatformInfo.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.csGinger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.csGinger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs
📚 Learning: 2025-08-22T14:57:03.711Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml:71-103
Timestamp: 2025-08-22T14:57:03.711Z
Learning: In Ginger POM wizard, prashelke prefers to show AI-powered features as visible to users but disabled by default (Visibility="Visible", IsEnabled="False") to promote feature discoverability rather than hiding them completely. Users should be able to see what AI capabilities are available even when not yet enabled.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
📚 Learning: 2025-09-02T07:51:28.799Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4289
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs:99-103
Timestamp: 2025-09-02T07:51:28.799Z
Learning: In Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs around line 99, prashelke prefers to keep the unused variable `selectedElementList` from GetSelectedElementList() call rather than removing it, despite it being unused in the current logic.
Applied to files:
Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs
📚 Learning: 2025-07-10T07:11:28.974Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695
Timestamp: 2025-07-10T07:11:28.974Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
🧬 Code graph analysis (4)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs (2)
GingerPlayConfiguration(23-240)IsGingerPlayConfigured(232-238)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WebPlatformInfo.cs (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
eElementType(6699-6739)ActUIElement(8992-9011)Ginger/GingerCoreCommon/UIElement/UIElementFilter.cs (3)
UIElementFilter(24-74)UIElementFilter(26-26)UIElementFilter(28-33)
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (1)
Ginger/GingerCoreCommon/UIElement/ElementLocator.cs (1)
ElementLocator(25-199)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs (2)
eElementType(302-307)eElementType(309-346)Ginger/GingerCoreCommon/Repository/ApplicationModelLib/POMModelLib/PomSetting.cs (1)
PomSetting(27-68)
🔇 Additional comments (11)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
433-438: LangVersion is adequate for C# 12 features
Project targets .NET 8, whose default LangVersion (“latestMajor”) maps to C# 12, and GingerCoreNET explicitly sets<LangVersion>preview</LangVersion>. No changes needed.Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WebPlatformInfo.cs (1)
289-296: Good: clear Svg filter description in Advanced section.Helpful guidance for users selecting SVG-related elements.
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (2)
626-634: LGTM: Locator grid “Learned Type” is correctly bound to ElementLocator.This complements the main grid column and will help users distinguish Manual/Auto/AI per locator.
346-355: No action required—ElementInfo already exposes LearnedType
TheGinger/GingerCoreCommon/UIElement/ElementInfo.csclass definespublic string LearnedType => LearnedTypeEnum.ToString();so the grid binding to “LearnedType” will succeed.
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (1)
672-673: Safe to update Unknown label — no code paths compare or special-case an emptyEnumValueDescriptionforeElementType.Unknown, so changing it to “Unknown” won’t alter existing UI/logic.Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (6)
5716-5718: Confirm intent: learning invisible SVG elementsAllowing SVG elements to bypass the visibility check will learn zero-sized or off-screen nodes. Confirm this is intentional and won’t bloat POMs on complex charts/diagrams. If needed, consider gating via a PomSetting flag.
6682-6684: LGTM: enabling TypeAtt override for HtmlNodeThis allows controlled coercion (e.g., SVG children) without depending on a native type attribute.
6699-6737: LGTM: map common SVG child tags to Svg when explicitly flaggedThe guarded mapping via TypeAtt.Equals("Svg", ignore case) avoids false positives.
10293-10294: LGTM: unify DoubleClick via DoUIElementClickReduces duplicated action-building logic.
10609-10624: LGTM: consolidated click pathsCentralized click/JS click/double-click/async-click improves maintainability.
Also applies to: 10635-10636
10645-10657: Unused helper: IsSvgElementByWebElementNo references found in this file. Remove if dead or wire it where needed (e.g., runtime detection for actions).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
6334-6352: Regression: non-SVG elements now use SVG XPath builderThe else branch now applies CreateSvgElementXPath to all non-SVG elements, breaking normal lookups. Distinguish root vs SVG child by tag name; use default XPath for non-SVG.
Apply this diff:
- string xpath = htmlElemNode.XPath; - if (elementType == eElementType.Svg) - { - if (!isShadowRootDetected) - { - xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); - } - } - else - { - // For SVG child elements, use local-name() in XPath - xpath = CreateSvgElementXPath(htmlElemNode); - } + string xpath; + if (elementType == eElementType.Svg) + { + var tag = htmlElemNode.Name.ToLower(); + if (tag == "svg") + { + // Root <svg> + xpath = isShadowRootDetected + ? htmlElemNode.XPath + : string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); + } + else + { + // SVG child + xpath = CreateSvgElementXPath(htmlElemNode); + } + } + else + { + // Non-SVG element + xpath = htmlElemNode.XPath; + }
8063-8072: Fix unescaped firstClass in contains(@Class) call (SeleniumDriver.cs line 8067)
Replace- conditions.Add($"contains(@class, '{firstClass}')"); + conditions.Add($"contains(@class, {EscapeXPathString(firstClass)})");No other occurrences of
contains(@class, '…')remain.Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
142-145: Log at WARN on subscription failure.This helps surface issues during wizard init/agent changes without flooding at DEBUG.
- Reporter.ToLog(eLogLevel.DEBUG, "Error subscribing to SeleniumDriver events", ex); + Reporter.ToLog(eLogLevel.WARN, "Error subscribing to SeleniumDriver events", ex);Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs (2)
156-162: Guard against null/empty filterList to avoid NREs.Defensive check makes this helper safe for broader reuse.
public void SelectElementsToList(ObservableList<UIElementFilter> elements, ObservableList<UIElementFilter> filterList) { + if (filterList == null || filterList.Count == 0) + { + foreach (var element in elements) + { + element.Selected = false; + } + return; + } foreach (UIElementFilter element in elements) { element.Selected = filterList.FirstOrDefault(f => f.ElementType == element.ElementType)?.Selected ?? false; } }
169-173: Remove unused MemoryStream in SaveLearnedPOM.Minor cleanup; avoids dead code.
- if (ScreenShot != null) - { - using (var ms = new MemoryStream()) - { - POM.ScreenShotImage = BitmapToBase64(ScreenShot); - } - } + if (ScreenShot != null) + { + POM.ScreenShotImage = BitmapToBase64(ScreenShot); + }
♻️ Duplicate comments (3)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
6016-6051: SVG locator strings: same double-quoting bugThe XPath literals built here also wrap EscapeXPathString with extra quotes and will break on quotes/special chars.
Apply this diff:
- LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-node-id='{EscapeXPathString(dataNodeId)}']", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-node-id={EscapeXPathString(dataNodeId)}]", ... - LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-backend-id='{EscapeXPathString(dataBackendId)}']", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-backend-id={EscapeXPathString(dataBackendId)}]", ... - LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and contains(@class, '{EscapeXPathString(cls)}')]", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and contains(@class, {EscapeXPathString(cls)})]",
6079-6114: Relative SVG XPath: parent/child attribute literals are over-quotedSame issue: don’t wrap EscapeXPathString output in quotes.
Apply this diff:
- parentSelector = $"*[@data-node-id='{EscapeXPathString(parentDataNodeId)}']"; + parentSelector = $"*[@data-node-id={EscapeXPathString(parentDataNodeId)}]"; ... - return $"//{parentSelector}//*[local-name()='{childSelector}' and @data-node-id='{EscapeXPathString(dataNodeId)}']"; + return $"//{parentSelector}//*[local-name()='{childSelector}' and @data-node-id={EscapeXPathString(dataNodeId)}]"; ... - return $"//{parentSelector}//*[local-name()='{childSelector}' and contains(@class, '{EscapeXPathString(firstClass)}')]"; + return $"//{parentSelector}//*[local-name()='{childSelector}' and contains(@class, {EscapeXPathString(firstClass)})]";Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
86-86: Unconditional binding for xLearnPOMByAI — good.This keeps the VM property always in sync as previously requested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs(1 hunks)Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs(3 hunks)Ginger/Ginger/UserControlsLib/UCElementDetails.xaml(1 hunks)Ginger/Ginger/UserControlsLib/UCElementDetails.xaml.cs(1 hunks)Ginger/GingerCoreCommon/UIElement/ElementInfo.cs(4 hunks)Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(12 hunks)
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1123-1126
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has removed an unnecessary empty line at 1123 in `WindowExplorerPage.xaml.cs` for better code compactness.
Applied to files:
Ginger/Ginger/UserControlsLib/UCElementDetails.xaml.cs
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.csGinger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.csGinger/GingerCoreCommon/UIElement/ElementInfo.csGinger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs
📚 Learning: 2025-09-04T09:34:04.271Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.csGinger/GingerCoreCommon/UIElement/ElementInfo.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs
📚 Learning: 2025-08-22T14:57:03.711Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml:71-103
Timestamp: 2025-08-22T14:57:03.711Z
Learning: In Ginger POM wizard, prashelke prefers to show AI-powered features as visible to users but disabled by default (Visibility="Visible", IsEnabled="False") to promote feature discoverability rather than hiding them completely. Users should be able to see what AI capabilities are available even when not yet enabled.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
📚 Learning: 2025-09-02T07:51:28.822Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4289
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs:99-103
Timestamp: 2025-09-02T07:51:28.822Z
Learning: In Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs around line 99, prashelke prefers to keep the unused variable `selectedElementList` from GetSelectedElementList() call rather than removing it, despite it being unused in the current logic.
Applied to files:
Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-16T14:17:56.429Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:89-93
Timestamp: 2025-07-16T14:17:56.429Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current casing for properties `insideIframe` and `insideShadowDOM` in the `ElementwrapperContext` class rather than changing them to PascalCase.
Applied to files:
Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T09:55:20.522Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-28T09:29:59.151Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml.cs:149-152
Timestamp: 2025-08-28T09:29:59.151Z
Learning: In Ginger's WebServicesDriverEditPage.xaml.cs, the FillComboItemsFromEnumType method should be used instead of FillComboFromEnumType when sorting by enum order is required. FillComboItemsFromEnumType preserves EnumValueDescription display through WPF's automatic ToString() calling on enum Content, and provides built-in sorting functionality via SortDescriptions.Add("text", Ascending). This was confirmed by AmanPrasad43 in PR #4286 for the ZAP vulnerability type dropdown.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:11:28.974Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695
Timestamp: 2025-07-10T07:11:28.974Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:12:52.786Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202
Timestamp: 2025-07-10T07:12:52.786Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: When searchContext is a ShadowRoot, convert XPath to a CSS selector via shadowDOM.ConvertXPathToCssSelector before calling FindElement, since ShadowRoot does not support XPath directly.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-12-04T11:45:57.024Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4017
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:11096-11096
Timestamp: 2024-12-04T11:45:57.024Z
Learning: In the `OnNetworkRequestSent` method in `SeleniumDriver.cs`, adding a null check for `_BrowserHelper` is not necessary.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-25T06:07:02.238Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#4146
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs:705-705
Timestamp: 2025-03-25T06:07:02.238Z
Learning: XPath expressions in the Ginger application should not use System.Security.SecurityElement.Escape() or similar methods to escape attribute values, as this might break the XPath functionality. The simple string replacement approach in POMLearner.cs is intentional.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-24T06:08:31.804Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4242
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:623-656
Timestamp: 2025-06-24T06:08:31.804Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs, the CSS selector construction by directly interpolating user input values in the GetElementLocator method is considered acceptable by the team and working as expected for their use cases.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-19T05:08:45.192Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:07:43.309Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserElement.cs:664-705
Timestamp: 2025-02-19T06:07:43.309Z
Learning: In the Ginger automation framework, exceptions in lower-level browser element operations are handled by their parent methods, which have more context about the operation being performed. Therefore, catch-and-rethrow without additional context is acceptable in the element-level methods.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-06-12T12:54:44.221Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3753
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:99-99
Timestamp: 2024-06-12T12:54:44.221Z
Learning: User IamRanjeetSingh prefers exceptions to propagate rather than being caught and handled locally within methods in `PlaywrightBrowserTab.cs`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:06:45.918Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:255-297
Timestamp: 2025-02-19T06:06:45.918Z
Learning: In the ActUIElementHandler class, error handling is done at the parent HandleAsync method level, making try-catch blocks in child methods unnecessary if they only rethrow exceptions.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-22T16:22:20.870Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:41-41
Timestamp: 2025-08-22T16:22:20.870Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current lowercase naming for properties like `elementGuid` rather than changing them to PascalCase, likely for consistency within the ElementWrapperInfo data model classes.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-30T07:40:08.756Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:358-365
Timestamp: 2025-01-30T07:40:08.756Z
Learning: In the Ginger automation framework, when an element is expected to exist but is not found, the code should throw an exception rather than return null, as the element's presence is a precondition for the operation.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
🧬 Code graph analysis (4)
Ginger/Ginger/UserControlsLib/UCElementDetails.xaml.cs (1)
Ginger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs (1)
InitViewItems(1416-1429)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs (2)
PomLearnUtils(41-472)PomLearnUtils(139-154)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (3)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs (2)
eElementType(302-307)eElementType(309-346)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ShadowDOM.cs (1)
ISearchContext(33-49)Ginger/GingerCoreCommon/Repository/ApplicationModelLib/POMModelLib/PomSetting.cs (1)
PomSetting(27-68)
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (2)
Ginger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs (2)
SetbtnDeleteHandler(855-859)ScrollToViewCurrentItem(759-765)Ginger/GingerCoreCommon/UIElement/ControlProperty.cs (1)
ControlProperty(26-48)
🔇 Additional comments (11)
Ginger/Ginger/UserControlsLib/UCElementDetails.xaml (1)
90-90: Keep Add/Delete visible (handlers are wired); remove or forward ScrollViewer attached props
- ucGrid exposes ShowAdd/ShowDelete (maps to btnAdd/btnDelete in Ginger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs) and pages attach handlers — e.g. PomElementsPage.xaml.cs wires btnAdd and calls SetbtnDeleteHandler. btnAdd_Click in ucGrid is intentionally left unimplemented for pages to hook. (Files: UCElementDetails.xaml, ucGrid.xaml.cs, PomElementsPage.xaml.cs.)
- ScrollViewer.VerticalScrollBarVisibility / CanContentScroll set on UCElementDetails.xaml’s ucGrid will not affect the inner DataGrid (ucGrid.xaml sets grdMain ScrollViewer.VerticalScrollBarVisibility="Auto"). Remove those attached props, wrap the ucGrid in a ScrollViewer, or add a forwarding DP in ucGrid to propagate the values to its inner DataGrid.
Likely an incorrect or invalid review comment.
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (2)
75-84: Notify dependent props — good fixRaising PropertyChanged for LearnedTypeEnum and LearnedType resolves stale bindings when IsAutoLearned changes.
564-578: LearnedType enum/properties added — align future AI state mappingCurrent mapping returns Manual/Auto only. If/when “AI” becomes material, ensure the computation updates accordingly and triggers notifications from the driving properties.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (3)
5716-5718: SVG visibility bypass — confirm behaviorYou now skip invisibility filtering for SVG roots but still filter SVG children later. Verify this asymmetry is intentional and won’t add hidden SVG roots while dropping their children. If it should be consistent, either allow both or filter both.
5731-5734: Good: processing SVG children within the current search contextPassing parentContext into ProcessSvgChildElements fixes iframe/shadow scope issues during POM learn.
10491-10496: Good: don’t swallow exceptions — set status and logSetting act.Status and act.Error and logging the exception makes failures visible and traceable.
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (2)
424-424: Enablement gating aligns with config and agent status — good.Keeps the control visible while disabling it until prerequisites are met.
213-221: Verify default XAML visibility of xLearnPOMByAI
I couldn’t locate an explicit declaration or default Visibility setting for xLearnPOMByAI in POMLearnConfigWizardPage.xaml via automated search. Please manually confirm in Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml that the control exists and note its default Visibility (e.g. “Collapsed” or “Visible”). This will ensure that always setting its visibility in code behind will behave as expected.Ginger/GingerCoreNET/Application Models/Learn/POM/PomLearnUtils.cs (1)
160-160: Enum comparison refactor — good.Using == for enums is clear and efficient.
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (2)
346-355: Learned Type column for elements — good.Correctly binds to ElementInfo.LearnedType with one-way mode.
839-841: Property grid handlers wiring — good.Uses Button.ClickEvent and delete handler via SetbtnDeleteHandler as intended.
Maheshkale447
requested changes
Sep 4, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (1)
106-121: Missing notifications when IsProcessed flips (affects LearnedType UI)If LearnedTypeEnum derives from IsProcessed, notify bindings here too.
if (_isProcessed != value) { _isProcessed = value; OnPropertyChanged(nameof(IsProcessed)); + OnPropertyChanged(nameof(LearnedTypeEnum)); + OnPropertyChanged(nameof(LearnedType)); }Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (2)
1149-1151: User-facing typo: “categoriess” → “categories”Fix the message string.
- Reporter.ToUser(eUserMsgKey.StaticInfoMessage, "No missing categoriess found."); + Reporter.ToUser(eUserMsgKey.StaticInfoMessage, "No missing categories found.");
1368-1368: WPF brush type mismatch (returns System.Drawing.Brush)Convert should return System.Windows.Media.Brush. Returning System.Drawing.Brush causes binding/runtime issues.
- return System.Drawing.Brushes.Gray; + return System.Windows.Media.Brushes.Gray;Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
6345-6358: SVG child branch is unreachable; distinguish root vs child tagsCurrent
elseruns only for non-Svgtypes. Route children by tag name instead.- if (elementType == eElementType.Svg) - { - if (!isShadowRootDetected) - { - xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); - } - } - else - { - // For SVG child elements, use local-name() in XPath - xpath = CreateSvgElementXPath(htmlElemNode); - } + if (elementType == eElementType.Svg) + { + if (string.Equals(htmlElemNode.Name, "svg", StringComparison.OrdinalIgnoreCase)) + { + if (!isShadowRootDetected) + { + xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); + } + } + else + { + // SVG child element + xpath = CreateSvgElementXPath(htmlElemNode); + } + }
8068-8097: Build valid robust relative XPath and escape class
- Escape the class fragment.
- Validate using the full XPath, not only the predicate.
- if (!string.IsNullOrWhiteSpace(firstClass) && !IsLikelyDynamic(firstClass)) - conditions.Add($"contains(@class, '{firstClass}')"); + if (!string.IsNullOrWhiteSpace(firstClass) && !IsLikelyDynamic(firstClass)) + conditions.Add($"contains(@class, {EscapeXPathString(firstClass)})"); @@ - string conditionString = conditions.Count > 0 ? $"[{string.Join(" and ", conditions)}]" : ""; - if (!string.IsNullOrEmpty(conditionString) && CheckElementLocateStatus(conditionString)) - { - var elementLocator = new ElementLocator() { LocateBy = eLocateBy.ByRelXPath, LocateValue = $"//{tag}{conditionString}", IsAutoLearned = true }; + string conditionString = conditions.Count > 0 ? $"[{string.Join(" and ", conditions)}]" : ""; + string candidate = $"//{tag}{conditionString}"; + if (!string.IsNullOrEmpty(conditionString) && CheckElementLocateStatus(candidate)) + { + var elementLocator = new ElementLocator() { LocateBy = eLocateBy.ByRelXPath, LocateValue = candidate, IsAutoLearned = true }; foundElemntInfo.Locators.Add(elementLocator); }
♻️ Duplicate comments (3)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (1)
564-578: LearnedType never returns AI — wire it when AI processing is presentIf IsProcessed indicates AI workflows, return AI and notify dependents. Minimal change:
public eLearnedType LearnedTypeEnum { - get - { - if (IsAutoLearned) { return eLearnedType.Auto; } - return eLearnedType.Manual; - } + get + { + if (IsProcessed) return eLearnedType.AI; + if (IsAutoLearned) return eLearnedType.Auto; + return eLearnedType.Manual; + } }Also raise notifications when IsProcessed flips (see next comment).
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
6028-6043: Fix double-quoting of escaped XPath literals (broken selectors)
EscapeXPathString(...)already returns a quoted literal; remove the extra quotes in XPath.- LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-node-id='{EscapeXPathString(dataNodeId)}']", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-node-id={EscapeXPathString(dataNodeId)}]", ... - LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-backend-id='{EscapeXPathString(dataBackendId)}']", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and @data-backend-id={EscapeXPathString(dataBackendId)}]", ... - LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and contains(@class, '{EscapeXPathString(cls)}')]", + LocateValue = $"//*[local-name()='{svgNode.Name.ToLower()}' and contains(@class, {EscapeXPathString(cls)})]",Also applies to: 6055-6061
6094-6124: Fix double-quoting in relative SVG XPathsSame issue in
CreateSvgRelativeXPath(...).- parentSelector = $"*[@data-node-id='{EscapeXPathString(parentDataNodeId)}']"; + parentSelector = $"*[@data-node-id={EscapeXPathString(parentDataNodeId)}]"; ... - return $"//{parentSelector}//*[local-name()='{childSelector}' and @data-node-id='{EscapeXPathString(dataNodeId)}']"; + return $"//{parentSelector}//*[local-name()='{childSelector}' and @data-node-id={EscapeXPathString(dataNodeId)}]"; ... - return $"//{parentSelector}//*[local-name()='{childSelector}' and contains(@class, '{EscapeXPathString(firstClass)}')]"; + return $"//{parentSelector}//*[local-name()='{childSelector}' and contains(@class, {EscapeXPathString(firstClass)})]";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs(3 hunks)Ginger/Ginger/UserControlsLib/UCElementDetails.xaml.cs(0 hunks)Ginger/GingerCoreCommon/UIElement/ElementInfo.cs(5 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(16 hunks)
💤 Files with no reviewable changes (1)
- Ginger/Ginger/UserControlsLib/UCElementDetails.xaml.cs
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.csGinger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T09:34:04.271Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2024-10-16T17:18:57.373Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#3959
File: Ginger/GingerCoreCommon/UIElement/ControlProperty.cs:34-41
Timestamp: 2024-10-16T17:18:57.373Z
Learning: Assignments to the `Category` property in `ControlProperty.cs` do not need to explicitly call `OnPropertyChanged`, as `OnPropertyChanged` is invoked within the property's setter. Therefore, explicit change notifications are unnecessary when setting this property elsewhere in the code.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs
📚 Learning: 2025-09-04T09:55:20.522Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs
📚 Learning: 2025-09-04T11:19:44.679Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:11:28.974Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695
Timestamp: 2025-07-10T07:11:28.974Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:12:52.786Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202
Timestamp: 2025-07-10T07:12:52.786Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: When searchContext is a ShadowRoot, convert XPath to a CSS selector via shadowDOM.ConvertXPathToCssSelector before calling FindElement, since ShadowRoot does not support XPath directly.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-12-04T11:45:57.024Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4017
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:11096-11096
Timestamp: 2024-12-04T11:45:57.024Z
Learning: In the `OnNetworkRequestSent` method in `SeleniumDriver.cs`, adding a null check for `_BrowserHelper` is not necessary.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-25T06:07:02.238Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#4146
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs:705-705
Timestamp: 2025-03-25T06:07:02.238Z
Learning: XPath expressions in the Ginger application should not use System.Security.SecurityElement.Escape() or similar methods to escape attribute values, as this might break the XPath functionality. The simple string replacement approach in POMLearner.cs is intentional.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-24T06:08:31.804Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4242
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:623-656
Timestamp: 2025-06-24T06:08:31.804Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs, the CSS selector construction by directly interpolating user input values in the GetElementLocator method is considered acceptable by the team and working as expected for their use cases.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-19T05:08:45.192Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:07:43.309Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserElement.cs:664-705
Timestamp: 2025-02-19T06:07:43.309Z
Learning: In the Ginger automation framework, exceptions in lower-level browser element operations are handled by their parent methods, which have more context about the operation being performed. Therefore, catch-and-rethrow without additional context is acceptable in the element-level methods.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-06-12T12:54:44.221Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3753
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:99-99
Timestamp: 2024-06-12T12:54:44.221Z
Learning: User IamRanjeetSingh prefers exceptions to propagate rather than being caught and handled locally within methods in `PlaywrightBrowserTab.cs`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:06:45.918Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:255-297
Timestamp: 2025-02-19T06:06:45.918Z
Learning: In the ActUIElementHandler class, error handling is done at the parent HandleAsync method level, making try-catch blocks in child methods unnecessary if they only rethrow exceptions.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-16T14:17:56.429Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:89-93
Timestamp: 2025-07-16T14:17:56.429Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current casing for properties `insideIframe` and `insideShadowDOM` in the `ElementwrapperContext` class rather than changing them to PascalCase.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-22T16:22:20.870Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:41-41
Timestamp: 2025-08-22T16:22:20.870Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current lowercase naming for properties like `elementGuid` rather than changing them to PascalCase, likely for consistency within the ElementWrapperInfo data model classes.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-02T07:51:28.822Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4289
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs:99-103
Timestamp: 2025-09-02T07:51:28.822Z
Learning: In Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs around line 99, prashelke prefers to keep the unused variable `selectedElementList` from GetSelectedElementList() call rather than removing it, despite it being unused in the current logic.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-30T07:40:08.756Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:358-365
Timestamp: 2025-01-30T07:40:08.756Z
Learning: In the Ginger automation framework, when an element is expected to exist but is not found, the code should throw an exception rather than return null, as the element's presence is a precondition for the operation.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3564
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:38-39
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `BaselineImage` and `VRTSavedBaseImageFilenameString` fields in `VRTAnalyzer.cs` should be declared as `const` to indicate their immutability.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it imports the ObservableList<> class which is used throughout the file.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-16T10:37:13.073Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-20T11:10:30.816Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:30.816Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-28T09:29:59.151Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml.cs:149-152
Timestamp: 2025-08-28T09:29:59.151Z
Learning: In Ginger's WebServicesDriverEditPage.xaml.cs, the FillComboItemsFromEnumType method should be used instead of FillComboFromEnumType when sorting by enum order is required. FillComboItemsFromEnumType preserves EnumValueDescription display through WPF's automatic ToString() calling on enum Content, and provides built-in sorting functionality via SortDescriptions.Add("text", Ascending). This was confirmed by AmanPrasad43 in PR #4286 for the ZAP vulnerability type dropdown.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
🧬 Code graph analysis (3)
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (3)
Ginger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs (2)
SetbtnDeleteHandler(855-859)ScrollToViewCurrentItem(759-765)Ginger/GingerCoreCommon/UIElement/ControlProperty.cs (1)
ControlProperty(26-48)Ginger/GingerCoreNET/GeneralLib/General.cs (3)
General(59-1487)List(63-71)List(190-252)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (4)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs (2)
eElementType(302-307)eElementType(309-346)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ShadowDOM.cs (2)
ISearchContext(33-49)ConvertXPathToCssSelector(64-89)Ginger/GingerCoreCommon/Repository/ApplicationModelLib/POMModelLib/PomSetting.cs (1)
PomSetting(27-68)Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (4)
ObservableList(503-506)ObservableList(508-511)ElementInfo(34-578)GetElementType(418-423)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
eElementType(6713-6753)
🔇 Additional comments (8)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (3)
75-84: Good: dependent property change notifications addedRaising PropertyChanged for LearnedTypeEnum and LearnedType fixes UI refresh issues when IsAutoLearned changes.
549-549: SVG icon mapping OKMapping Image or Svg to eImageType.Image is fine. If a dedicated SVG icon exists later, swap it in without changing callers.
731-733: Enum label for SVG looks goodEnumValueDescription("SVG") added correctly for display consistency.
Ginger/Ginger/ApplicationModelsLib/POMModels/PomElementsPage.xaml.cs (2)
346-355: Correct binding to ElementInfo.LearnedTypeSwitching the grid column to ElementInfo.LearnedType with OneWay binding is correct and matches the row item type.
839-841: Property grid handlers are correctly attachedAdd and Delete handlers registered once during view setup. Looks good.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (3)
5732-5735: Good: SVG children searched within the current contextPropagating
parentContextand honoring frame/shadow is correct.
5790-5797: Helper is fine as-isKeeping
IsSvgElement(eElementType)aligns with maintainer preference.
5821-5831: Good: ShadowRoot-safe lookup for SVG childrenConverting XPath to CSS when
searchContext is ShadowRootis correct.Also applies to: 5833-5836
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
6343-6357: Bug: non-SVG elements now incorrectly use SVG XPath builderThe else-branch sets XPath via CreateSvgElementXPath for all non-SVG nodes. This breaks learning for regular HTML tags. Distinguish root vs child SVG; default non-SVG to htmlElemNode.XPath.
Apply:
- if (elementType == eElementType.Svg) - { - if (!isShadowRootDetected) - { - xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); - } - } - else - { - // For SVG child elements, use local-name() in XPath - xpath = CreateSvgElementXPath(htmlElemNode); - } + if (elementType == eElementType.Svg) + { + var tag = htmlElemNode.Name.ToLower(); + if (tag == "svg") + { + if (!isShadowRootDetected) + { + xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); + } + } + else + { + // SVG child element: use local-name() + xpath = CreateSvgElementXPath(htmlElemNode); + } + } + else + { + // Non‑SVG element + xpath = htmlElemNode.XPath; + }
♻️ Duplicate comments (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
8328-8350: SVG spy path: OK to keep WebElement-based helper (team preference)Using IsSvgElementByWebElement here is acceptable per prior agreement; enhanced properties + XPath generation look fine.
5977-6061: Remove wrapping quotes around EscapeXPathString calls in XPath locators
All occurrences where EscapeXPathString is enclosed in single quotes (e.g.@data-node-id='{EscapeXPathString(...)}orcontains(@class, '{EscapeXPathString(...)}')) must be updated to omit the extra quotes—use@data-node-id={EscapeXPathString(...)}andcontains(@class, {EscapeXPathString(...)})instead. Affects lines 5977-6061, 6078-6134 and 9182-9223.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(15 hunks)
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
📚 Learning: 2025-09-04T09:55:20.522Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T11:19:44.679Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T15:42:00.287Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T15:43:57.773Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:11:28.974Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695
Timestamp: 2025-07-10T07:11:28.974Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T09:34:04.271Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:12:52.786Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202
Timestamp: 2025-07-10T07:12:52.786Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: When searchContext is a ShadowRoot, convert XPath to a CSS selector via shadowDOM.ConvertXPathToCssSelector before calling FindElement, since ShadowRoot does not support XPath directly.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-12-04T11:45:57.024Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4017
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:11096-11096
Timestamp: 2024-12-04T11:45:57.024Z
Learning: In the `OnNetworkRequestSent` method in `SeleniumDriver.cs`, adding a null check for `_BrowserHelper` is not necessary.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-25T06:07:02.238Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#4146
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs:705-705
Timestamp: 2025-03-25T06:07:02.238Z
Learning: XPath expressions in the Ginger application should not use System.Security.SecurityElement.Escape() or similar methods to escape attribute values, as this might break the XPath functionality. The simple string replacement approach in POMLearner.cs is intentional.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-24T06:08:31.804Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4242
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:623-656
Timestamp: 2025-06-24T06:08:31.804Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs, the CSS selector construction by directly interpolating user input values in the GetElementLocator method is considered acceptable by the team and working as expected for their use cases.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-19T05:08:45.192Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:07:43.309Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserElement.cs:664-705
Timestamp: 2025-02-19T06:07:43.309Z
Learning: In the Ginger automation framework, exceptions in lower-level browser element operations are handled by their parent methods, which have more context about the operation being performed. Therefore, catch-and-rethrow without additional context is acceptable in the element-level methods.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-06-12T12:54:44.221Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3753
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:99-99
Timestamp: 2024-06-12T12:54:44.221Z
Learning: User IamRanjeetSingh prefers exceptions to propagate rather than being caught and handled locally within methods in `PlaywrightBrowserTab.cs`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:06:45.918Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:255-297
Timestamp: 2025-02-19T06:06:45.918Z
Learning: In the ActUIElementHandler class, error handling is done at the parent HandleAsync method level, making try-catch blocks in child methods unnecessary if they only rethrow exceptions.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-16T14:17:56.429Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:89-93
Timestamp: 2025-07-16T14:17:56.429Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current casing for properties `insideIframe` and `insideShadowDOM` in the `ElementwrapperContext` class rather than changing them to PascalCase.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-22T16:22:20.870Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:41-41
Timestamp: 2025-08-22T16:22:20.870Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current lowercase naming for properties like `elementGuid` rather than changing them to PascalCase, likely for consistency within the ElementWrapperInfo data model classes.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-02T07:51:28.822Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4289
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs:99-103
Timestamp: 2025-09-02T07:51:28.822Z
Learning: In Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs around line 99, prashelke prefers to keep the unused variable `selectedElementList` from GetSelectedElementList() call rather than removing it, despite it being unused in the current logic.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-30T07:40:08.756Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:358-365
Timestamp: 2025-01-30T07:40:08.756Z
Learning: In the Ginger automation framework, when an element is expected to exist but is not found, the code should throw an exception rather than return null, as the element's presence is a precondition for the operation.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3564
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:38-39
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `BaselineImage` and `VRTSavedBaseImageFilenameString` fields in `VRTAnalyzer.cs` should be declared as `const` to indicate their immutability.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
🧬 Code graph analysis (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (4)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs (2)
eElementType(302-307)eElementType(309-346)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ShadowDOM.cs (2)
ISearchContext(33-49)ConvertXPathToCssSelector(64-89)Ginger/GingerCoreCommon/Repository/ApplicationModelLib/POMModelLib/PomSetting.cs (1)
PomSetting(27-68)Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (4)
ObservableList(503-506)ObservableList(508-511)ElementInfo(34-578)GetElementType(418-423)
🔇 Additional comments (5)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (5)
5731-5734: Good: SVG children searched in current context (frame/shadow) and ShadowRoot-safe XPath→CSS conversionPassing ISearchContext and converting XPath to CSS for ShadowRoot fixes iframe/shadow failures and aligns with prior guidance.
Also applies to: 5798-5831
6171-6184: OK: transform/translate extractionRegex for translate(...) is fine for collecting UI properties.
11155-11160: Good: don’t swallow exceptions; set status and logSetting act.Status/act.Error and logging the exception improves diagnostics.
10961-10961: Use of Actions.DoubleClick for double-clicksSwitching to native DoubleClick improves clarity and timing correctness.
Also applies to: 11291-11294
5977-6017: Fix XPath: don’t wrap EscapeXPathString output in extra quotesEscapeXPathString returns a quoted literal. Additional quotes produce invalid XPath (e.g., ''foo''). Remove the outer quotes.
- foreach (var className in classes) - { - conditions.Add($"contains(@class, {EscapeXPathString(className)})"); - } + foreach (var className in classes) + { + conditions.Add($"contains(@class, {EscapeXPathString(className)})"); + } @@ - else if (attr == "href" && value.StartsWith("#")) - { - conditions.Add($"@{attr}={EscapeXPathString(value)}"); - } + else if (attr == "href" && value.StartsWith("#")) + { + conditions.Add($"@{attr}={EscapeXPathString(value)}"); + } @@ - else if (!IsLikelyDynamic(value)) - { - conditions.Add($"@{attr}={EscapeXPathString(value)}"); - } + else if (!IsLikelyDynamic(value)) + { + conditions.Add($"@{attr}={EscapeXPathString(value)}"); + }⛔ Skipped due to learnings
Learnt from: prashelke PR: Ginger-Automation/Ginger#4249 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202 Timestamp: 2025-07-10T07:12:52.786Z Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.Learnt from: Maheshkale447 PR: Ginger-Automation/Ginger#4146 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs:705-705 Timestamp: 2025-03-25T06:07:02.238Z Learning: XPath expressions in the Ginger application should not use System.Security.SecurityElement.Escape() or similar methods to escape attribute values, as this might break the XPath functionality. The simple string replacement approach in POMLearner.cs is intentional.Learnt from: prashelke PR: Ginger-Automation/Ginger#4249 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695 Timestamp: 2025-07-10T07:11:28.974Z Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.Learnt from: prashelke PR: Ginger-Automation/Ginger#4294 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291 Timestamp: 2025-09-04T15:43:57.773Z Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.Learnt from: prashelke PR: Ginger-Automation/Ginger#4207 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:0-0 Timestamp: 2025-05-09T18:13:31.764Z Learning: When splitting strings in C#, use `new string[] { "/" }` for array initialization, and prefer clear variable names like `xpathSplitter` (with double 't').
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
6351-6363: Scope SVG-specific XPath logic exclusively to SVG elements
TheGetWebElementmethod currently misappliesCreateSvgElementXPathto all non-SVG types, breaking locators for standard HTML elements. Invert the branch so non-SVG elements retainhtmlElemNode.XPathand SVG elements (root vs. children) use the specialized XPath:- if (elementType == eElementType.Svg) - { - if (!isShadowRootDetected) - { - xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); - } - } - else - { - // For SVG child elements, use local-name() in XPath - xpath = CreateSvgElementXPath(htmlElemNode); - } + if (elementType == eElementType.Svg) + { + var tag = htmlElemNode.Name.ToLowerInvariant(); + if (tag == "svg") + { + if (!isShadowRootDetected) + xpath = string.Concat(htmlElemNode.ParentNode.XPath, "//*[local-name()='svg']"); + } + else + { + // SVG child element: use local-name() in XPath + xpath = CreateSvgElementXPath(htmlElemNode); + } + }Verify that standard HTML elements (e.g.,
<button>) use their original XPath and nested SVG children (e.g.,<path>) resolve via thelocal-name()-based builder.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(15 hunks)
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
📚 Learning: 2025-09-04T15:43:57.773Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T15:42:00.287Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T09:55:20.522Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T11:19:44.679Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T16:40:09.493Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:6019-6061
Timestamp: 2025-09-04T16:40:09.493Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (AddSvgSpecificLocators, around lines ~6000–6065) for PR Ginger-Automation/Ginger#4294, maintainer prashelke indicated that adjusting XPath to remove outer quotes around EscapeXPathString(...) is not required. Do not suggest changing the quoting around EscapeXPathString for SVG-specific locators in future reviews unless maintainers request it.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T16:40:33.050Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9182-9223
Timestamp: 2025-09-04T16:40:33.050Z
Learning: In PR Ginger-Automation/Ginger#4294 (SeleniumDriver.cs GenerateSvgAttributeBasedXPath), maintainer declined changing XPath to remove extra quotes around EscapeXPathString(...) results; keep current quoting as-is in this area for now.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:12:52.786Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202
Timestamp: 2025-07-10T07:12:52.786Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-10T07:11:28.974Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:4684-4695
Timestamp: 2025-07-10T07:11:28.974Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, attribute values used in CSS selectors (e.g., ByTitle, ByAriaLabel, ByDataTestId, ByPlaceholder) should be escaped using a helper method (such as EscapeCssAttributeValue) to prevent selector breakage due to special characters.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T10:00:39.282Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: When searchContext is a ShadowRoot, convert XPath to a CSS selector via shadowDOM.ConvertXPathToCssSelector before calling FindElement, since ShadowRoot does not support XPath directly.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-12-04T11:45:57.024Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4017
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:11096-11096
Timestamp: 2024-12-04T11:45:57.024Z
Learning: In the `OnNetworkRequestSent` method in `SeleniumDriver.cs`, adding a null check for `_BrowserHelper` is not necessary.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-25T06:07:02.238Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#4146
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMLearner.cs:705-705
Timestamp: 2025-03-25T06:07:02.238Z
Learning: XPath expressions in the Ginger application should not use System.Security.SecurityElement.Escape() or similar methods to escape attribute values, as this might break the XPath functionality. The simple string replacement approach in POMLearner.cs is intentional.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-24T06:08:31.804Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4242
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:623-656
Timestamp: 2025-06-24T06:08:31.804Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs, the CSS selector construction by directly interpolating user input values in the GetElementLocator method is considered acceptable by the team and working as expected for their use cases.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-06-19T05:08:45.192Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:07:43.309Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserElement.cs:664-705
Timestamp: 2025-02-19T06:07:43.309Z
Learning: In the Ginger automation framework, exceptions in lower-level browser element operations are handled by their parent methods, which have more context about the operation being performed. Therefore, catch-and-rethrow without additional context is acceptable in the element-level methods.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-06-12T12:54:44.221Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3753
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Playwright/PlaywrightBrowserTab.cs:99-99
Timestamp: 2024-06-12T12:54:44.221Z
Learning: User IamRanjeetSingh prefers exceptions to propagate rather than being caught and handled locally within methods in `PlaywrightBrowserTab.cs`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-02-19T06:06:45.918Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4107
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:255-297
Timestamp: 2025-02-19T06:06:45.918Z
Learning: In the ActUIElementHandler class, error handling is done at the parent HandleAsync method level, making try-catch blocks in child methods unnecessary if they only rethrow exceptions.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-07-16T14:17:56.429Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:89-93
Timestamp: 2025-07-16T14:17:56.429Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current casing for properties `insideIframe` and `insideShadowDOM` in the `ElementwrapperContext` class rather than changing them to PascalCase.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-22T16:22:20.870Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:41-41
Timestamp: 2025-08-22T16:22:20.870Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current lowercase naming for properties like `elementGuid` rather than changing them to PascalCase, likely for consistency within the ElementWrapperInfo data model classes.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-04T09:34:04.271Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-30T07:40:08.756Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:358-365
Timestamp: 2025-01-30T07:40:08.756Z
Learning: In the Ginger automation framework, when an element is expected to exist but is not found, the code should throw an exception rather than return null, as the element's presence is a precondition for the operation.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-09-02T07:51:28.822Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4289
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs:99-103
Timestamp: 2025-09-02T07:51:28.822Z
Learning: In Ginger/GingerCoreNET/Application Models/Delta/PomDelta/PomDeltaUtils.cs around line 99, prashelke prefers to keep the unused variable `selectedElementList` from GetSelectedElementList() call rather than removing it, despite it being unused in the current logic.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3564
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:38-39
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `BaselineImage` and `VRTSavedBaseImageFilenameString` fields in `VRTAnalyzer.cs` should be declared as `const` to indicate their immutability.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
🔇 Additional comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
5802-5853: LGTM: SVG children resolved within the active context (ShadowRoot-safe)Passing
ISearchContext searchContextand converting XPath→CSS whensearchContext is ShadowRootaligns with prior guidance and fixes iframe/shadow lookups. Good change.
Maheshkale447
approved these changes
Sep 5, 2025
Maheshkale447
approved these changes
Sep 5, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (1)
731-733: EnumValueDescription("SVG") added — verify icon mapping existsThe label looks good. Please verify GetElementTypeImage maps Svg; in this file’s switch it currently falls back to Element.
Add (or confirm) this branch in GetElementTypeImage:
case eElementType.Svg: return eImageType.Image; // or a dedicated SVG icon if available
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5731-5734
Timestamp: 2025-09-04T10:00:39.282Z
Learning: In SeleniumDriver.ProcessSvgChildElements, always search SVG children within the current ISearchContext (frame/shadow) instead of using Driver.FindElement, and pass parentContext from FindAllElementsFromPOM.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
📚 Learning: 2025-09-04T09:34:04.271Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.271Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T11:19:44.679Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.679Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T15:42:00.287Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.287Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T09:55:20.522Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.522Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-08-28T09:29:59.151Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml.cs:149-152
Timestamp: 2025-08-28T09:29:59.151Z
Learning: In Ginger's WebServicesDriverEditPage.xaml.cs, the FillComboItemsFromEnumType method should be used instead of FillComboFromEnumType when sorting by enum order is required. FillComboItemsFromEnumType preserves EnumValueDescription display through WPF's automatic ToString() calling on enum Content, and provides built-in sorting functionality via SortDescriptions.Add("text", Ascending). This was confirmed by AmanPrasad43 in PR #4286 for the ZAP vulnerability type dropdown.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T15:43:57.773Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.773Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-04T16:40:09.493Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:6019-6061
Timestamp: 2025-09-04T16:40:09.493Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (AddSvgSpecificLocators, around lines ~6000–6065) for PR Ginger-Automation/Ginger#4294, maintainer prashelke indicated that adjusting XPath to remove outer quotes around EscapeXPathString(...) is not required. Do not suggest changing the quoting around EscapeXPathString for SVG-specific locators in future reviews unless maintainers request it.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
📚 Learning: 2025-09-05T09:30:10.044Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5942-5960
Timestamp: 2025-09-05T09:30:10.044Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around SVG helpers, approx. lines 5942–5960 and 5983–5984), maintainer prefers keeping ToLower() for tag/name normalization; do not suggest replacing with ToLowerInvariant() in future reviews for this area.
Applied to files:
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs
🔇 Additional comments (2)
Ginger/GingerCoreCommon/UIElement/ElementInfo.cs (2)
75-84: Dependent property notifications added — LGTMRaising PropertyChanged for LearnedTypeEnum and LearnedType along with IsAutoLearned fixes the binding refresh issue.
758-759: No-op whitespaceNo action needed.
Comment on lines
+563
to
578
|
|
||
| public enum eLearnedType { Manual, Auto, AI } | ||
|
|
||
| // Non-breaking addition: | ||
| public eLearnedType LearnedTypeEnum | ||
| { | ||
| get | ||
| { | ||
| if (IsAutoLearned) { return eLearnedType.Auto; } | ||
| return eLearnedType.Manual; | ||
| } | ||
| } | ||
|
|
||
| // Keep existing string property as-is, or derive from the enum: | ||
| public string LearnedType => LearnedTypeEnum.ToString(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make LearnedType UI text robust; keep AI reserved, and plan notifications if used later
Optional: add EnumValueDescription to eLearnedType and derive LearnedType via the common helper so display strings are consistent/localizable. If a future rule sets AI (per maintainer note, AI must be retained), remember to raise notifications from any property that affects it (e.g., IsProcessed).
- public enum eLearnedType { Manual, Auto, AI }
+ public enum eLearnedType
+ {
+ [EnumValueDescription("Manual")] Manual,
+ [EnumValueDescription("Auto")] Auto,
+ [EnumValueDescription("AI")] AI
+ }
@@
- public string LearnedType => LearnedTypeEnum.ToString();
+ public string LearnedType =>
+ Amdocs.Ginger.Common.GeneralLib.General.GetEnumValueDescription(typeof(eLearnedType), LearnedTypeEnum);Note: If/when LearnedTypeEnum can become AI (e.g., based on IsProcessed), ensure IsProcessed setter also calls OnPropertyChanged(nameof(LearnedTypeEnum)) and OnPropertyChanged(nameof(LearnedType)).
(Confirmed: AI value should be kept for future features per maintainers’ guidance.)
📝 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.
Suggested change
| public enum eLearnedType { Manual, Auto, AI } | |
| // Non-breaking addition: | |
| public eLearnedType LearnedTypeEnum | |
| { | |
| get | |
| { | |
| if (IsAutoLearned) { return eLearnedType.Auto; } | |
| return eLearnedType.Manual; | |
| } | |
| } | |
| // Keep existing string property as-is, or derive from the enum: | |
| public string LearnedType => LearnedTypeEnum.ToString(); | |
| } | |
| public enum eLearnedType | |
| { | |
| [EnumValueDescription("Manual")] Manual, | |
| [EnumValueDescription("Auto")] Auto, | |
| [EnumValueDescription("AI")] AI | |
| } | |
| // Non-breaking addition: | |
| public eLearnedType LearnedTypeEnum | |
| { | |
| get | |
| { | |
| if (IsAutoLearned) { return eLearnedType.Auto; } | |
| return eLearnedType.Manual; | |
| } | |
| } | |
| // Derive the UI text via the localization helper instead of ToString() | |
| public string LearnedType => | |
| Amdocs.Ginger.Common.GeneralLib.General.GetEnumValueDescription(typeof(eLearnedType), LearnedTypeEnum); |
🤖 Prompt for AI Agents
In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs around lines 563 to 578, the
LearnedType UI text should be robust and localizable while preserving the
reserved AI enum value and ensuring property-change notifications when related
state changes; update eLearnedType to include EnumValueDescription attributes
for each member, change LearnedType to derive its display string via the
existing common helper that reads EnumValueDescription (instead of ToString()),
and ensure any setter that can cause LearnedTypeEnum to change (e.g.,
IsProcessed or IsAutoLearned) calls OnPropertyChanged(nameof(LearnedTypeEnum))
and OnPropertyChanged(nameof(LearnedType)) so the UI updates and future AI state
changes will trigger notifications.
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Improvements