Get Value by OCR enhancement#4296
Merged
Merged
Conversation
Contributor
WalkthroughAdds OCR-based GetValueByOCR action: captures element screenshots, runs OCR, and returns text; brings element windows to foreground (with optional resize). Wires the action through enums, drivers, platform action lists, and exposes OCR helper method. Includes minor formatting/usings tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WindowsDriver as WindowsDriver
participant UIHelper as UIElementOperationsHelper
participant WinAPI as Window Manager
participant OCR as GingerOCR
User->>WindowsDriver: Execute ActUIElement(GetValueByOCR)
WindowsDriver->>UIHelper: GetValueByOCR(AutomationElement)
UIHelper->>WinAPI: BringElementWindowToForeground(+optional resize)
WinAPI-->>UIHelper: Window foregrounded
UIHelper->>UIHelper: Capture element bounding-rect → PNG bytes
UIHelper->>OCR: ReadTextFromByteArray(PNG bytes)
OCR-->>UIHelper: OCR text
UIHelper-->>WindowsDriver: ActionResult(outputValue=OCR text)
WindowsDriver-->>User: Return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (11)📚 Learning: 2025-08-25T08:48:11.915ZApplied to files:
📚 Learning: 2025-09-04T15:43:57.773ZApplied to files:
📚 Learning: 2025-04-08T12:31:45.924ZApplied to files:
📚 Learning: 2025-09-05T09:30:10.044ZApplied to files:
📚 Learning: 2025-09-04T09:55:20.522ZApplied to files:
📚 Learning: 2025-07-10T07:11:28.974ZApplied to files:
📚 Learning: 2025-09-04T11:19:44.679ZApplied to files:
📚 Learning: 2025-06-16T10:37:13.073ZApplied to files:
📚 Learning: 2025-04-25T13:29:45.059ZApplied to files:
📚 Learning: 2025-04-25T13:29:45.059ZApplied to files:
📚 Learning: 2025-03-20T11:10:33.780ZApplied to files:
🧬 Code graph analysis (1)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (2)
🔇 Additional comments (2)
✨ 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
921-931: Show output grid for GetValueByOCR results.Currently only GetValue returns true; OCR reads won’t show Actual value in the grid.
Apply:
return ElementAction switch { eElementAction.Click or eElementAction.Collapse or eElementAction.Expand => false, - eElementAction.GetValue => true, + eElementAction.GetValue or eElementAction.GetValueByOCR => true, _ => false, };Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
4127-4156: Guard the STA clipboard fallback with a timeoutThe while (bDone == false) loop can hang if the thread deadlocks. Add a bounded wait.
- t.SetApartmentState(ApartmentState.STA); - t.Start(); - while (bDone == false) - { - Thread.Sleep(100); - } + t.SetApartmentState(ApartmentState.STA); + t.Start(); + var sw = Stopwatch.StartNew(); + while (!bDone && sw.ElapsedMilliseconds < 3000) + { + Thread.Sleep(50); + } + if (!bDone) + { + Reporter.ToLog(eLogLevel.DEBUG, "Fallback clipboard read timed out after 3 seconds."); + }
📜 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 (10)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs(4 hunks)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs(4 hunks)Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs(2 hunks)Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs(2 hunks)Ginger/GingerCoreNET/ActionsLib/ActASCFControl.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(5 hunks)Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PlatformInfoBase.cs(1 hunks)Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PowerBuilderPlatform.cs(1 hunks)Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WindowsPlatform.cs(10 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 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/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.csGinger/GingerCoreNET/ActionsLib/ActASCFControl.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/GingerCoreCommon/Actions/UI Element/ActUIElement.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCoreCommon/Actions/UI Element/ActUIElement.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCoreCommon/Actions/UI Element/ActUIElement.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCoreCommon/Actions/UI Element/ActUIElement.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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-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-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-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-04-08T12:31:45.924Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4169
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:321-336
Timestamp: 2025-04-08T12:31:45.924Z
Learning: The GetTextLengthAsync method in Ginger Automation's Web driver should attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue) to calculate text length, providing enhanced capabilities to read text length from different types of elements.
Applied to files:
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
🧬 Code graph analysis (8)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PlatformInfoBase.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
ActUIElement(8590-8609)eElementType(6299-6337)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PowerBuilderPlatform.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
ActUIElement(8590-8609)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WindowsPlatform.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (2)
ActUIElement(8590-8609)eElementType(6299-6337)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateReturnParamActual(1308-1335)
Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (1)
UIElementOperationsHelper(42-944)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (3)
Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateReturnParamActual(1308-1335)
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (19)
UIAuto(371-399)UIAuto(696-713)UIAuto(715-737)UIAuto(739-771)UIAuto(831-848)UIAuto(1010-1039)UIAuto(1110-1145)UIAuto(1215-1384)UIAuto(1387-1391)UIAuto(1491-1510)UIAuto(1532-1578)UIAuto(1580-1616)UIAuto(1618-1644)UIAuto(3049-3060)UIAuto(3659-3689)UIAuto(5055-5058)Bitmap(4911-4914)Bitmap(4936-4947)Bitmap(6332-6352)Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)
Ginger/GingerCoreNET/ActionsLib/ActASCFControl.cs (2)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreNET/ActionsLib/Java/ActJavaElement.cs (2)
ActUIElement(244-272)ActUIElement(286-326)
🔇 Additional comments (9)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
27-29: Usings look correct.System and System.ComponentModel are needed for List and [Description] attributes.
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PlatformInfoBase.cs (1)
240-241: GetValueByOCR handlers implemented for all target drivers
Verified implementation in: UIElementOperationsHelper (core), WindowsDriver, UIAComWrapperHelper (UIA), PowerBuilderPlatform, SeleniumDriver + ActUIElementHandler (web) and enumeration mappings. No unsupported driver paths—action visibility controlled accordingly.Ginger/GingerCoreNET/ActionsLib/ActASCFControl.cs (2)
84-85: Mapping added to new UIElement action — looks good.Obsolete ASCF action now correctly maps to ActUIElement.GetValueByOCR.
397-400: Enum ordinal usage verified safe
Search across the codebase found no(int)eControlActioncasts or</>comparisons, and all switch-cases and lists reference explicit enum members. Adding the new value at 19 won’t break existing logic.Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
5850-5851: LGTM: await remaining batch updateFlushing the remaining queued elements asynchronously is correct and prevents drops.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (1)
111-113: Wiring looks correct.The new switch case is routed properly.
Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs (1)
1174-1174: No-op formatting change.Whitespace only; safe to ignore.
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/WindowsPlatform.cs (1)
95-95: Confirm action surface area for GetValueByOCR across element types.Adding GetValueByOCR broadly makes sense if OCR works generically on bounding rectangles. Please confirm this is intended for all these types (e.g., TabItem, TreeItem, HyperLink, Document) and won’t clutter action menus unnecessarily.
Also applies to: 124-124, 133-133, 141-141, 149-149, 161-161, 180-180, 190-190, 200-200, 208-208, 220-220, 228-228, 269-269, 278-278
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
5322-5332: No-op changeThis looks like whitespace-only around the Equals case; nothing to act on.
Comment on lines
+22
to
+27
| using GingerCore.GingerOCR; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.Drawing; | ||
| using System.IO; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Confirm OCR and drawing deps; add OS guard import
System.Drawing APIs are Windows-only on .NET 6+. If this project multi-targets, add a runtime/compile guard. Also add the import needed for OS checks.
Apply:
using GingerCore.GingerOCR;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Drawing;
using System.IO;
+using System.Runtime.InteropServices;Please confirm target frameworks include -windows TFMs where needed (e.g., net6.0-windows).
📝 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
| using GingerCore.GingerOCR; | |
| using System; | |
| using System.Collections.Generic; | |
| using System.Diagnostics; | |
| using System.Drawing; | |
| using System.IO; | |
| using GingerCore.GingerOCR; | |
| using System; | |
| using System.Collections.Generic; | |
| using System.Diagnostics; | |
| using System.Drawing; | |
| using System.IO; | |
| using System.Runtime.InteropServices; |
🤖 Prompt for AI Agents
In Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs around lines
22-27 the file uses System.Drawing which is Windows-only on .NET 6+; add the
required import (using System.Runtime.InteropServices) and protect
System.Drawing usage with either a compile-time TFM guard (ensure project
targets include -windows TFMs like net6.0-windows or net7.0-windows and wrap
Windows-only code with #if WINDOWS / #endif or appropriate NET6_0_OR_GREATER &&
WINDOWS symbols) or a runtime OS check
(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) around drawing calls;
update project TargetFramework(s) to include -windows TFMs if not already and,
if using runtime guard, log/throw a clear error when running on non-Windows so
callers know the functionality is unavailable.
Comment on lines
+574
to
+619
| public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) | ||
| { | ||
| ActionResult actionResult = new ActionResult(); | ||
| string tempImagePath = null; | ||
| try | ||
| { | ||
| BringElementWindowToForeground(automationElement); | ||
| // 1. Get bounding rectangle | ||
| Rectangle rect = automationElement.Current.BoundingRectangle; | ||
| //Rectangle rectangle = new System.Drawing.Rectangle(rect.X, rect.Y, rect.Width, rect.Height); | ||
| int left = (int)rect.Left; | ||
| int top = (int)rect.Top; | ||
| int width = (int)(rect.Right - rect.Left); | ||
| int height = (int)(rect.Bottom - rect.Top); | ||
| // 2. Capture screenshot | ||
| using (var bmp = new Bitmap(width, height)) | ||
| { | ||
| using (var g = Graphics.FromImage(bmp)) | ||
| { | ||
| g.CopyFromScreen(left, top, 0, 0, rect.Size); | ||
| } | ||
|
|
||
| // 3. Save to temp file | ||
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | ||
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | ||
| } | ||
|
|
||
| // 4. OCR | ||
| string ocrText = GingerOcrOperations.ReadTextFromImage(tempImagePath); | ||
| actionResult.outputValue = ocrText; | ||
| actionResult.executionInfo = "OCR value extracted successfully"; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| actionResult.errorMessage = "Failed to get value by OCR: " + ex.Message; | ||
| } | ||
| finally | ||
| { | ||
| // 5. Clean up temp file | ||
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | ||
| { | ||
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | ||
| } | ||
| } | ||
| return actionResult; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix compile/runtime issues in GetValueByOCR (rect type mismatch, zero-size/off-screen, OS guard)
Current code assigns UIA bounding rect to System.Drawing.Rectangle and uses rect.Size; this likely won’t compile and will throw for zero/negative sizes or non-Windows.
Apply:
- public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement)
+ public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement)
{
ActionResult actionResult = new ActionResult();
string tempImagePath = null;
+ // Windows-only guard
+ if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+ {
+ actionResult.errorMessage = "GetValueByOCR is supported on Windows only.";
+ return actionResult;
+ }
+ if (automationElement == null)
+ {
+ actionResult.errorMessage = "Automation element is null.";
+ return actionResult;
+ }
try
{
BringElementWindowToForeground(automationElement);
// 1. Get bounding rectangle
- Rectangle rect = automationElement.Current.BoundingRectangle;
- //Rectangle rectangle = new System.Drawing.Rectangle(rect.X, rect.Y, rect.Width, rect.Height);
- int left = (int)rect.Left;
- int top = (int)rect.Top;
- int width = (int)(rect.Right - rect.Left);
- int height = (int)(rect.Bottom - rect.Top);
+ var rect = automationElement.Current.BoundingRectangle;
+ int left = rect.X;
+ int top = rect.Y;
+ int width = rect.Width;
+ int height = rect.Height;
+ if (width <= 0 || height <= 0)
+ {
+ actionResult.errorMessage = "Element has zero-sized bounding rectangle.";
+ return actionResult;
+ }
// 2. Capture screenshot
using (var bmp = new Bitmap(width, height))
{
using (var g = Graphics.FromImage(bmp))
{
- g.CopyFromScreen(left, top, 0, 0, rect.Size);
+ g.CopyFromScreen(left, top, 0, 0, new Size(width, height));
}
// 3. Save to temp file
tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
}
// 4. OCR
string ocrText = GingerOcrOperations.ReadTextFromImage(tempImagePath);
- actionResult.outputValue = ocrText;
+ actionResult.outputValue = string.IsNullOrWhiteSpace(ocrText) ? ocrText : ocrText.Trim();
actionResult.executionInfo = "OCR value extracted successfully";
}Notes:
- For occluded/off-screen windows, CopyFromScreen may still fail; consider switching to PrintWindow-based capture like WindowToBitmap for robustness (optional).
Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+4650
to
4651
| Reporter.ToLog(eLogLevel.ERROR, $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", currentPOMElementInfo?.Locators?.Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'") ?? Enumerable.Empty<string>())}]", ex); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix null-conditional chain: can throw before null-coalescing kicks in
currentPOMElementInfo?.Locators?.Where(...) will call Where on null when Locators is null. Coalesce before LINQ.
Apply:
- Reporter.ToLog(eLogLevel.ERROR, $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", currentPOMElementInfo?.Locators?.Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'") ?? Enumerable.Empty<string>())}]", ex);
+ Reporter.ToLog(
+ eLogLevel.ERROR,
+ $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", (currentPOMElementInfo?.Locators ?? Enumerable.Empty<ElementLocator>()).Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'"))}]",
+ ex);📝 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
| Reporter.ToLog(eLogLevel.ERROR, $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", currentPOMElementInfo?.Locators?.Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'") ?? Enumerable.Empty<string>())}]", ex); | |
| } | |
| Reporter.ToLog( | |
| eLogLevel.ERROR, | |
| $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", (currentPOMElementInfo?.Locators ?? Enumerable.Empty<ElementLocator>()).Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'"))}]", | |
| ex); | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
around lines 4650-4651, the expression
currentPOMElementInfo?.Locators?.Where(...) can throw because Where is invoked
on a null Locators; coalesce Locators to an empty sequence before calling LINQ
(e.g. replace currentPOMElementInfo?.Locators?.Where(...) with
(currentPOMElementInfo?.Locators ?? Enumerable.Empty<LocatorType>()).Where(...),
using the actual locator type), then keep the string.Join and logging as-is so
ActiveLocators builds from an empty sequence instead of throwing.
Comment on lines
+9700
to
+9717
| case ActUIElement.eElementAction.GetValueByOCR: | ||
| if (act.ElementType == eElementType.HyperLink) | ||
| { | ||
| if (e != null) | ||
| { | ||
| act.AddOrUpdateReturnParamActual("Actual", e.GetAttribute("href")); | ||
| } | ||
| else | ||
| { | ||
| act.AddOrUpdateReturnParamActual("Actual", ""); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| act.AddOrUpdateReturnParamActual("Actual", GetElementValue(e)); | ||
| } | ||
| break; | ||
|
|
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
GetValueByOCR in Selenium does not perform OCR; clarify behavior or fall back
This duplicates GetValue logic and may mislead users expecting OCR. Prefer explicit fallback to GetValue (with a note) or mark unsupported.
Apply one of:
- Fallback with clear intent:
- case ActUIElement.eElementAction.GetValueByOCR:
- if (act.ElementType == eElementType.HyperLink)
- {
- if (e != null)
- {
- act.AddOrUpdateReturnParamActual("Actual", e.GetAttribute("href"));
- }
- else
- {
- act.AddOrUpdateReturnParamActual("Actual", "");
- }
- }
- else
- {
- act.AddOrUpdateReturnParamActual("Actual", GetElementValue(e));
- }
- break;
+ case ActUIElement.eElementAction.GetValueByOCR:
+ Reporter.ToLog(eLogLevel.INFO, "GetValueByOCR is not supported by Selenium; falling back to DOM value.");
+ goto case ActUIElement.eElementAction.GetValue;- Or, if intended to be unsupported on Web, skip with status:
+ case ActUIElement.eElementAction.GetValueByOCR:
+ act.Status = Amdocs.Ginger.CoreNET.Execution.eRunStatus.Skipped;
+ act.ExInfo = "GetValueByOCR is not supported by Selenium driver.";
+ break;If there is a shared OCR helper (e.g., UIElementOperationsHelper), wire it here instead.
🏁 Script executed:
#!/bin/bash
# Search for occurrences of OCR-related logic and helpers
rg -n "GetValueByOCR" -C2
rg -n "case ActUIElement.eElementAction.GetValueByOCR" -C2
rg -n "OCR" -C2
rg -n "UIElementOperationsHelper" -C2
rg -n "Tesseract" -C2
rg -n "OCR" Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs -C2Length of output: 72980
SeleniumDriver.GetValueByOCR must not silently duplicate GetValue
In SeleniumDriver.cs at the ActUIElement.eElementAction.GetValueByOCR case (around line 9700), the code simply falls back to DOM value without OCR, which is misleading. Replace with one of the suggested patterns:
goto case ActUIElement.eElementAction.GetValuewith an INFO log about the fallback- mark the action as Skipped with a clear “not supported” status
- or call the shared OCR helper if true OCR support is intended
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
around lines 9700-9717, the GetValueByOCR case currently returns the DOM value
(duplicating GetValue) which is misleading; change it to one of the three
acceptable fixes: 1) perform a fallback by jumping to the GetValue case (use
"goto case ActUIElement.eElementAction.GetValue") and emit an INFO log stating
OCR is not available and DOM value is used; 2) mark the action as
Skipped/NotSupported by setting the action status (e.g., act.Status =
eRunStatus.Skipped or equivalent), set a clear return param like "Actual" = ""
and add a descriptive message that OCR is not supported for this element; or 3)
if true OCR support exists, call the shared OCR helper method here and populate
the return param from its result; implement only one of these options
consistently, ensure a clear log/return message is present, and remove the
silent DOM fallback.
Comment on lines
+163
to
164
| ActUIElement.eElementAction.GetValueByOCR, | ||
| ]; |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify PB support before exposing OCR as a validation type.
PB actions list comments indicate very limited support; adding GetValueByOCR here may surfacing a non-functional option if driver handling is missing.
- If unsupported, drop it from this list or guard by capability.
- Script to check PB-side handling:
🏁 Script executed:
#!/bin/bash
# Look for PB driver/handlers for OCR value retrieval
rg -n -C2 --type cs 'PowerBuilder|PB' Ginger | rg -n -C2 --type cs 'GetValueByOCR'Length of output: 82
🏁 Script executed:
#!/bin/bash
# Search for any references to GetValueByOCR in C# files
rg -n "GetValueByOCR" -g '*.cs' -C2
# Search for any OCR mentions in C# code to see if there’s related support
rg -n "OCR" -g '*.cs' -C2Length of output: 45468
Guard or remove OCR action in PowerBuilderPlatform
PowerBuilder driver contains no handler for GetValueByOCR, so exposing it here will surface a non-functional option. In Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PowerBuilderPlatform.cs (lines 163–164), either drop ActUIElement.eElementAction.GetValueByOCR or wrap it behind a capability-check before adding it to the list.
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/PowerBuilderPlatform.cs
around lines 163-164, the list currently includes
ActUIElement.eElementAction.GetValueByOCR which the PowerBuilder driver does not
support; either remove this enum entry from the actions list or conditionally
add it only when the PowerBuilder driver reports OCR capability. Update the code
to check a capability flag or driver feature method (e.g., driver.SupportsOCR or
platform.Capabilities.HasFlag(OCR)) before pushing GetValueByOCR, or simply
delete the GetValueByOCR entry from the array if no OCR support exists for
PowerBuilder.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
4128-4157: Avoid potential indefinite wait in clipboard fallbackThe STA thread loop lacks a timeout; add a bounded wait to prevent hangs under clipboard contention.
Apply this diff:
private static string GetClipboardText() { string clipboardText = GingerCore.General.GetClipboardText(); if (string.IsNullOrEmpty(clipboardText)) { Reporter.ToLog(eLogLevel.DEBUG, "Clipboard couldn't be identified, so trying alternative."); - bool bDone = false; + bool bDone = false; // Fall-back mechanism, if above GingerCore.General.GetClipboardText doesn't work, then var t = new Thread(() => { try { clipboardText = Clipboard.GetText(TextDataFormat.Text); bDone = true; } catch (Exception ex) { Reporter.ToLog(eLogLevel.DEBUG, "Exception in GetClipboardText", ex); bDone = true; } }); t.SetApartmentState(ApartmentState.STA); t.Start(); - while (bDone == false) - { - Thread.Sleep(100); - } + var sw = Stopwatch.StartNew(); + while (!bDone && sw.ElapsedMilliseconds < 2000) + { + Thread.Sleep(50); + } + if (!bDone) + { + Reporter.ToLog(eLogLevel.DEBUG, "Clipboard fallback timed out after 2 seconds."); + } } return clipboardText; }
♻️ Duplicate comments (4)
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (3)
22-27: Windows-only dependency: add OS runtime guard importSystem.Drawing is Windows-only on .NET 6+. Add RuntimeInformation import to support the runtime guard in OCR code.
using GingerCore.GingerOCR; using System; using System.Collections.Generic; using System.Diagnostics; using System.Drawing; using System.IO; +using System.Runtime.InteropServices;
923-952: Foreground reliability: resolve top-level window and poll instead of fixed sleepShowWindow on a child element can be a no-op; replace fixed Sleep with a short poll on focus/visibility.
public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null) { if (automationElement == null) return; try { - // Bring window to foreground - WinAPIAutomation.ShowWindow(automationElement); + // Resolve top-level window + var windowElement = automationElement; + var walker = UIAuto.TreeWalker.ControlViewWalker; + while (windowElement != null && windowElement.Current.ControlType != UIAuto.ControlType.Window) + { + var parent = walker.GetParent(windowElement); + if (parent == null || parent == UIAuto.AutomationElement.RootElement) break; + windowElement = parent; + } + if (windowElement == null) windowElement = automationElement; + + // Bring window to foreground + WinAPIAutomation.ShowWindow(windowElement); // Optionally resize if (resizeWidth.HasValue && resizeHeight.HasValue) { - WinAPIAutomation.ResizeExternalWindow(automationElement, resizeWidth.Value, resizeHeight.Value); + WinAPIAutomation.ResizeExternalWindow(windowElement, resizeWidth.Value, resizeHeight.Value); } - // Give time to the window to come to the front - Thread.Sleep(500); + // Poll up to ~1.5s until foreground/focus instead of fixed sleep + var sw = Stopwatch.StartNew(); + while (sw.ElapsedMilliseconds < 1500) + { + if (windowElement.Current.HasKeyboardFocus || windowElement.Current.IsOffscreen == false) + { + break; + } + Thread.Sleep(50); + } } catch (Exception ex) { Reporter.ToLog(eLogLevel.ERROR, "Failed to bring window to foreground", ex); } }
574-624: Guard OCR path for non-Windows and null element to avoid PlatformNotSupportedExceptionAdd OS check and null check at the top of GetValueByOCR. Without this, System.Drawing usage will throw on non-Windows.
public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) { ActionResult actionResult = new ActionResult(); string tempImagePath = null; + // Windows-only guard + null check + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + actionResult.errorMessage = "GetValueByOCR is supported on Windows only."; + return actionResult; + } + if (automationElement == null) + { + actionResult.errorMessage = "Automation element is null."; + return actionResult; + } try { BringElementWindowToForeground(automationElement); // 1. Get bounding rectangle Rectangle rect = automationElement.Current.BoundingRectangle;Optional: consider capturing the top-level window via PrintWindow and cropping to the element rectangle to reduce failures when occluded.
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
2453-2464: GetValueByOCR path incorrectly uses non‑OCR value retrievalThis still calls GetControlValue(AE). Route via OCR.
Apply this diff:
case ActUIElement.eElementAction.GetValueByOCR: if (AE == null) { return false; } - if (GetControlValue(AE) == validationValue) + var actual = GetControlValueByOCR(AE)?.Trim(); + var expected = validationValue?.Trim(); + if (string.Equals(actual, expected, StringComparison.Ordinal)) { return true; } break;
📜 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 (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs(6 hunks)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-01-15T13:04:25.188Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#4058
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:613-633
Timestamp: 2025-01-15T13:04:25.188Z
Learning: In the Ginger automation framework, UI validation is the primary mechanism for validating input types (click types, locate by types, validation types). The error message checks in the code serve as a secondary validation layer.
Applied to files:
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.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/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
📚 Learning: 2025-04-08T12:31:45.924Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4169
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:321-336
Timestamp: 2025-04-08T12:31:45.924Z
Learning: The GetTextLengthAsync method in Ginger Automation's Web driver should attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue) to calculate text length, providing enhanced capabilities to read text length from different types of elements.
Applied to files:
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
📚 Learning: 2025-03-20T11:10:33.780Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:33.780Z
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/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
🧬 Code graph analysis (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (21)
System(568-591)System(6942-6953)UIAuto(372-400)UIAuto(697-714)UIAuto(716-738)UIAuto(740-772)UIAuto(832-849)UIAuto(1011-1040)UIAuto(1111-1146)UIAuto(1216-1385)UIAuto(1388-1392)UIAuto(1492-1511)UIAuto(1533-1579)UIAuto(1581-1617)UIAuto(1619-1645)UIAuto(3050-3061)UIAuto(3660-3690)UIAuto(5095-5098)Bitmap(4951-4954)Bitmap(4976-4987)Bitmap(6372-6392)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)
🔇 Additional comments (3)
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (1)
867-885: LGTM: OCR validation flow is correctEarly exit on null, fallback to GetText, and Trim()+Ordinal compare are appropriate.
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (2)
29-29: OCR namespace import looks correctNo concerns with adding GingerCore.GingerOCR here.
2211-2211: No action neededLine is an unchanged control-flow break.
Comment on lines
2212
to
2221
| case ActUIElement.eElementAction.GetValueByOCR: | ||
| if (AE == null) | ||
| { | ||
| return false; | ||
| } | ||
| if (GetControlValueByOCR(AE) == validationValue) | ||
| { | ||
| return true; | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Trim/normalize OCR text before comparing to avoid false negatives
Direct equality with raw OCR often fails due to trailing spaces/newlines.
Apply this diff:
case ActUIElement.eElementAction.GetValueByOCR:
if (AE == null)
{
return false;
}
- if (GetControlValueByOCR(AE) == validationValue)
+ var actual = GetControlValueByOCR(AE)?.Trim();
+ var expected = validationValue?.Trim();
+ if (string.Equals(actual, expected, StringComparison.Ordinal))
{
return true;
}
break;🏁 Script executed:
#!/bin/bash
file="Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs"
if [ ! -f "$file" ]; then
echo "File not found: $file"
exit 1
fi
# Show the OCR comparison snippet around lines 2212–2221
sed -n '2190,2230p' "$file"
# Locate the definition of GetControlValueByOCR to confirm its return type
grep -R -n "GetControlValueByOCR(" "$file"
grep -R -n -C5 "GetControlValueByOCR" "$file"Length of output: 2584
🏁 Script executed:
#!/bin/bash
# Display lines around the start of the validation handler to locate validationValue declaration
sed -n '2120,2180p' Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csLength of output: 2490
Trim OCR output and expected value before comparison
GetControlValueByOCR returns raw text and validationValue may include trailing whitespace or newlines.
- if (GetControlValueByOCR(AE) == validationValue)
+ var actual = GetControlValueByOCR(AE)?.Trim();
+ var expected = validationValue?.Trim();
+ if (string.Equals(actual, expected, StringComparison.Ordinal))📝 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
| case ActUIElement.eElementAction.GetValueByOCR: | |
| if (AE == null) | |
| { | |
| return false; | |
| } | |
| if (GetControlValueByOCR(AE) == validationValue) | |
| { | |
| return true; | |
| } | |
| break; | |
| case ActUIElement.eElementAction.GetValueByOCR: | |
| if (AE == null) | |
| { | |
| return false; | |
| } | |
| var actual = GetControlValueByOCR(AE)?.Trim(); | |
| var expected = validationValue?.Trim(); | |
| if (string.Equals(actual, expected, StringComparison.Ordinal)) | |
| { | |
| return true; | |
| } | |
| break; |
🤖 Prompt for AI Agents
In Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs around lines
2212-2221, the comparison uses raw OCR output and validationValue which may
include trailing/leading whitespace or newlines; trim both strings (and handle
nulls) before comparing — e.g., retrieve the OCR string and validationValue,
call .Trim() (or use string.IsNullOrWhiteSpace checks) on each, then compare the
trimmed results (preferably with string.Equals(..., StringComparison.Ordinal) or
similar) and return true only if the trimmed values match.
Comment on lines
+4236
to
+4272
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | ||
| { | ||
| string tempImagePath = null; | ||
| try | ||
| { | ||
| var rect = element.Current.BoundingRectangle; | ||
| var rectangle = new System.Drawing.Rectangle( | ||
| rect.X, rect.Y, rect.Width, rect.Height); | ||
|
|
||
| using (var bmp = new Bitmap(rectangle.Width, rectangle.Height)) | ||
| { | ||
| using (var g = Graphics.FromImage(bmp)) | ||
| { | ||
| g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size); | ||
| } | ||
| // 3. Save to temp file | ||
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | ||
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | ||
|
|
||
| return GingerOcrOperations.ReadTextFromImage(tempImagePath); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex); | ||
| } | ||
| finally | ||
| { | ||
| // 5. Clean up temp file | ||
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | ||
| { | ||
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | ||
| } | ||
| } | ||
| // fallback: use normal value getter | ||
| return GetControlValue(element); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden OCR capture: handle off‑screen/zero‑size/clamp to virtual screen and foreground window; trim result
Improves robustness and accuracy of OCR screenshots.
Apply this diff:
- protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element)
- {
- string tempImagePath = null;
- try
- {
- var rect = element.Current.BoundingRectangle;
- var rectangle = new System.Drawing.Rectangle(
- rect.X, rect.Y, rect.Width, rect.Height);
-
- using (var bmp = new Bitmap(rectangle.Width, rectangle.Height))
- {
- using (var g = Graphics.FromImage(bmp))
- {
- g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size);
- }
- // 3. Save to temp file
- tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
- bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
-
- return GingerOcrOperations.ReadTextFromImage(tempImagePath);
- }
- }
- catch (Exception ex)
- {
- Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex);
- }
- finally
- {
- // 5. Clean up temp file
- if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath))
- {
- try { File.Delete(tempImagePath); } catch { /* ignore */ }
- }
- }
- // fallback: use normal value getter
- return GetControlValue(element);
- }
+ protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element)
+ {
+ if (element == null || !IsWindowValid(element))
+ {
+ return string.Empty;
+ }
+
+ string tempImagePath = null;
+ try
+ {
+ // Ensure window is foreground to minimize occlusion in CopyFromScreen
+ try { WinAPIAutomation.ShowWindow(CurrentWindow); } catch { /* best effort */ }
+
+ var r = element.Current.BoundingRectangle;
+ if (r.Width <= 0 || r.Height <= 0)
+ {
+ return GetControlValue(element);
+ }
+
+ // Clamp to virtual screen to avoid CopyFromScreen exceptions on multi‑monitor/negative coords
+ var vs = System.Windows.Forms.SystemInformation.VirtualScreen;
+ var src = System.Drawing.Rectangle.Intersect(new System.Drawing.Rectangle(r.X, r.Y, r.Width, r.Height), vs);
+ if (src.Width <= 0 || src.Height <= 0)
+ {
+ return GetControlValue(element);
+ }
+
+ using (var bmp = new Bitmap(src.Width, src.Height))
+ using (var g = Graphics.FromImage(bmp))
+ {
+ g.CopyFromScreen(src.Location, System.Drawing.Point.Empty, src.Size);
+ tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
+ bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
+ }
+
+ var text = GingerOcrOperations.ReadTextFromImage(tempImagePath);
+ return text?.Trim();
+ }
+ catch (Exception ex)
+ {
+ Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex);
+ }
+ finally
+ {
+ if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath))
+ {
+ try { File.Delete(tempImagePath); } catch { /* ignore */ }
+ }
+ }
+ return GetControlValue(element);
+ }📝 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
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | |
| { | |
| string tempImagePath = null; | |
| try | |
| { | |
| var rect = element.Current.BoundingRectangle; | |
| var rectangle = new System.Drawing.Rectangle( | |
| rect.X, rect.Y, rect.Width, rect.Height); | |
| using (var bmp = new Bitmap(rectangle.Width, rectangle.Height)) | |
| { | |
| using (var g = Graphics.FromImage(bmp)) | |
| { | |
| g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size); | |
| } | |
| // 3. Save to temp file | |
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | |
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | |
| return GingerOcrOperations.ReadTextFromImage(tempImagePath); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex); | |
| } | |
| finally | |
| { | |
| // 5. Clean up temp file | |
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | |
| { | |
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | |
| } | |
| } | |
| // fallback: use normal value getter | |
| return GetControlValue(element); | |
| } | |
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | |
| { | |
| if (element == null || !IsWindowValid(element)) | |
| { | |
| return string.Empty; | |
| } | |
| string tempImagePath = null; | |
| try | |
| { | |
| // Ensure window is foreground to minimize occlusion in CopyFromScreen | |
| try { WinAPIAutomation.ShowWindow(CurrentWindow); } catch { /* best effort */ } | |
| var r = element.Current.BoundingRectangle; | |
| if (r.Width <= 0 || r.Height <= 0) | |
| { | |
| return GetControlValue(element); | |
| } | |
| // Clamp to virtual screen to avoid CopyFromScreen exceptions on multi-monitor/negative coords | |
| var vs = System.Windows.Forms.SystemInformation.VirtualScreen; | |
| var src = System.Drawing.Rectangle.Intersect( | |
| new System.Drawing.Rectangle(r.X, r.Y, r.Width, r.Height), | |
| vs); | |
| if (src.Width <= 0 || src.Height <= 0) | |
| { | |
| return GetControlValue(element); | |
| } | |
| using (var bmp = new Bitmap(src.Width, src.Height)) | |
| using (var g = Graphics.FromImage(bmp)) | |
| { | |
| g.CopyFromScreen(src.Location, System.Drawing.Point.Empty, src.Size); | |
| tempImagePath = Path.Combine( | |
| Path.GetTempPath(), | |
| $"GingerOCR_{Guid.NewGuid()}.png"); | |
| bmp.Save( | |
| tempImagePath, | |
| System.Drawing.Imaging.ImageFormat.Png); | |
| } | |
| var text = GingerOcrOperations.ReadTextFromImage(tempImagePath); | |
| return text?.Trim(); | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog( | |
| eLogLevel.DEBUG, | |
| "GetValueByOCR fallback to GetControlValue", | |
| ex); | |
| } | |
| finally | |
| { | |
| if (!string.IsNullOrEmpty(tempImagePath) | |
| && File.Exists(tempImagePath)) | |
| { | |
| try { File.Delete(tempImagePath); } | |
| catch { /* ignore */ } | |
| } | |
| } | |
| return GetControlValue(element); | |
| } |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
4131-4157: Prevent potential hang in clipboard fallbackAdd a bounded wait to avoid spinning indefinitely if the STA thread never signals completion.
- t.SetApartmentState(ApartmentState.STA); - t.Start(); - while (bDone == false) - { - Thread.Sleep(100); - } + t.SetApartmentState(ApartmentState.STA); + t.Start(); + var sw = Stopwatch.StartNew(); + while (!bDone && sw.ElapsedMilliseconds < 2000) // max 2s + { + Thread.Sleep(50); + }
♻️ Duplicate comments (4)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
2453-2464: Bug: GetValueByOCR still compares non-OCR value in SelectFromPaneThis branch uses GetControlValue(AE) instead of the OCR path, so OCR is never exercised here. Align with LocateAndValidateElement.
Apply this diff:
- case ActUIElement.eElementAction.GetValueByOCR: + case ActUIElement.eElementAction.GetValueByOCR: if (AE == null) { return false; } - if (GetControlValue(AE) == validationValue) + if (GetControlValueByOCR(AE) == validationValue) { return true; } break;To ensure there are no similar leftovers elsewhere, run:
#!/bin/bash # Find any GetValueByOCR branches still using GetControlValue( rg -nP -C2 '(case\s+ActUIElement\.eElementAction\.GetValueByOCR:)(?s).*?GetControlValue\s*\(' --type csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (3)
867-885: Good: early exit on null and trimmed compare.This prevents NRE and avoids whitespace mismatches. Thanks for addressing the earlier feedback.
22-27: Add OS guard import (System.Drawing is Windows-only on .NET 6+).Include RuntimeInformation to enable a Windows-only runtime guard in OCR code. This mirrors prior feedback.
using GingerCore.GingerOCR; using System; using System.Collections.Generic; using System.Diagnostics; using System.Drawing; using System.IO; +using System.Runtime.InteropServices;
574-624: Guard non-Windows and null; keep OCR robust.Without an OS check, CopyFromScreen will crash on non-Windows targets; null element should also short-circuit.
- public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) + public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) { ActionResult actionResult = new ActionResult(); string tempImagePath = null; + // OS/runtime guard + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + actionResult.errorMessage = "GetValueByOCR is supported on Windows only."; + return actionResult; + } + if (automationElement == null) + { + actionResult.errorMessage = "Automation element is null."; + return actionResult; + } try { BringElementWindowToForeground(automationElement); // 1. Get bounding rectangle - Rectangle rect = automationElement.Current.BoundingRectangle; + Rectangle rect = automationElement.Current.BoundingRectangle; int left = rect.Left; int top = rect.Top; int width = rect.Width; int height = rect.Height; if (width <= 0 || height <= 0) { actionResult.errorMessage = "Element has zero-sized bounding rectangle."; return actionResult; } // 2. Capture screenshot using (var bmp = new Bitmap(width, height)) { using (var g = Graphics.FromImage(bmp)) { g.CopyFromScreen(left, top, 0, 0, new Size(width, height)); } // 3. Save to temp file tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); } // 4. OCR string ocrText = GingerOcrOperations.ReadTextFromImage(tempImagePath); actionResult.outputValue = string.IsNullOrWhiteSpace(ocrText) ? ocrText : ocrText.Trim(); actionResult.executionInfo = "OCR value extracted successfully"; } catch (Exception ex) { actionResult.errorMessage = "Failed to get value by OCR: " + ex.Message; } finally { // 5. Clean up temp file if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) { try { File.Delete(tempImagePath); } catch { /* ignore */ } } } return actionResult; }
📜 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 (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs(6 hunks)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 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/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
📚 Learning: 2025-04-08T12:31:45.924Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4169
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:321-336
Timestamp: 2025-04-08T12:31:45.924Z
Learning: The GetTextLengthAsync method in Ginger Automation's Web driver should attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue) to calculate text length, providing enhanced capabilities to read text length from different types of elements.
Applied to files:
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs
📚 Learning: 2025-03-20T11:10:33.780Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:33.780Z
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/GingerCore/Drivers/Common/UIElementOperationsHelper.cs
🧬 Code graph analysis (2)
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (3)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (21)
System(568-591)System(6942-6953)UIAuto(372-400)UIAuto(697-714)UIAuto(716-738)UIAuto(740-772)UIAuto(832-849)UIAuto(1011-1040)UIAuto(1111-1146)UIAuto(1216-1385)UIAuto(1388-1392)UIAuto(1492-1511)UIAuto(1533-1579)UIAuto(1581-1617)UIAuto(1619-1645)UIAuto(3050-3061)UIAuto(3660-3690)UIAuto(5095-5098)Bitmap(4951-4954)Bitmap(4976-4987)Bitmap(6372-6392)Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs (2)
Bitmap(1598-1601)Bitmap(1955-1958)Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (3)
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (1)
ActUIElement(33-1070)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs (4)
UIAuto(669-704)UIAuto(1677-1713)UIAuto(1715-1741)UIAuto(1742-1751)
🔇 Additional comments (2)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (2)
29-29: LGTM: OCR namespace import addedThe new using directive is appropriate for the OCR integration.
2212-2221: LGTM: LocateAndValidateElement now routes OCR through GetControlValueByOCRThe action branch correctly calls the OCR path and preserves the AE null-guard.
Comment on lines
+4236
to
+4272
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | ||
| { | ||
| string tempImagePath = null; | ||
| try | ||
| { | ||
| var rect = element.Current.BoundingRectangle; | ||
| var rectangle = new System.Drawing.Rectangle( | ||
| rect.X, rect.Y, rect.Width, rect.Height); | ||
|
|
||
| using (var bmp = new Bitmap(rectangle.Width, rectangle.Height)) | ||
| { | ||
| using (var g = Graphics.FromImage(bmp)) | ||
| { | ||
| g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size); | ||
| } | ||
| // 3. Save to temp file | ||
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | ||
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | ||
|
|
||
| return GingerOcrOperations.ReadTextFromImage(tempImagePath); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex); | ||
| } | ||
| finally | ||
| { | ||
| // 5. Clean up temp file | ||
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | ||
| { | ||
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | ||
| } | ||
| } | ||
| // fallback: use normal value getter | ||
| return GetControlValue(element); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden OCR capture: foreground, occlusion-safe capture, and screen clipping
Without foregrounding/paint and clipping, CopyFromScreen can read stale/occluded pixels, yielding flaky OCR. Prefer window-capture + crop, then fall back to screen copy, and trim output.
protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element)
{
- string tempImagePath = null;
- try
- {
- var rect = element.Current.BoundingRectangle;
- var rectangle = new System.Drawing.Rectangle(
- rect.X, rect.Y, rect.Width, rect.Height);
-
- using (var bmp = new Bitmap(rectangle.Width, rectangle.Height))
- {
- using (var g = Graphics.FromImage(bmp))
- {
- g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size);
- }
- // 3. Save to temp file
- tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
- bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
-
- return GingerOcrOperations.ReadTextFromImage(tempImagePath);
- }
- }
- catch (Exception ex)
- {
- Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex);
- }
- finally
- {
- // 5. Clean up temp file
- if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath))
- {
- try { File.Delete(tempImagePath); } catch { /* ignore */ }
- }
- }
- // fallback: use normal value getter
- return GetControlValue(element);
+ string tempImagePath = null;
+ try
+ {
+ if (element == null) return string.Empty;
+
+ // 1) Bring the window to the foreground and force a paint
+ try
+ {
+ var status = WinAPIAutomation.SetForeGroundWindow(CurrentWindow.Current.ProcessId);
+ if (!status) { WinAPIAutomation.ShowWindow(CurrentWindow); }
+ ForcePaint(CurrentWindow.Current.NativeWindowHandle);
+ }
+ catch { /* best-effort */ }
+
+ var er = element.Current.BoundingRectangle;
+ if (er.Width <= 0 || er.Height <= 0) return GetControlValue(element);
+
+ // Prefer occlusion-safe window capture, then crop to element bounds
+ using (var winBmp = WindowToBitmap(CurrentWindow))
+ {
+ if (winBmp != null)
+ {
+ var wr = CurrentWindow.Current.BoundingRectangle;
+ var crop = new System.Drawing.Rectangle(er.X - wr.X, er.Y - wr.Y, er.Width, er.Height);
+ crop = System.Drawing.Rectangle.Intersect(crop, new System.Drawing.Rectangle(0, 0, winBmp.Width, winBmp.Height));
+ if (crop.Width > 0 && crop.Height > 0)
+ {
+ using (var bmp = winBmp.Clone(crop, winBmp.PixelFormat))
+ {
+ tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
+ bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
+ return GingerOcrOperations.ReadTextFromImage(tempImagePath)?.Trim();
+ }
+ }
+ }
+ }
+
+ // 2) Fallback: CopyFromScreen with clipping to virtual screen
+ var vs = System.Windows.Forms.SystemInformation.VirtualScreen;
+ var screenRect = System.Drawing.Rectangle.Intersect(
+ new System.Drawing.Rectangle(er.X, er.Y, er.Width, er.Height),
+ new System.Drawing.Rectangle(vs.X, vs.Y, vs.Width, vs.Height));
+ if (screenRect.Width <= 0 || screenRect.Height <= 0) return GetControlValue(element);
+
+ using (var bmp = new Bitmap(screenRect.Width, screenRect.Height))
+ using (var g = Graphics.FromImage(bmp))
+ {
+ g.CopyFromScreen(new System.Drawing.Point(screenRect.X, screenRect.Y), System.Drawing.Point.Empty, screenRect.Size);
+ tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png");
+ bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png);
+ return GingerOcrOperations.ReadTextFromImage(tempImagePath)?.Trim();
+ }
+ }
+ catch (Exception ex)
+ {
+ Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex);
+ }
+ finally
+ {
+ if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath))
+ {
+ try { File.Delete(tempImagePath); } catch { /* ignore */ }
+ }
+ }
+ return GetControlValue(element);
}📝 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
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | |
| { | |
| string tempImagePath = null; | |
| try | |
| { | |
| var rect = element.Current.BoundingRectangle; | |
| var rectangle = new System.Drawing.Rectangle( | |
| rect.X, rect.Y, rect.Width, rect.Height); | |
| using (var bmp = new Bitmap(rectangle.Width, rectangle.Height)) | |
| { | |
| using (var g = Graphics.FromImage(bmp)) | |
| { | |
| g.CopyFromScreen(rectangle.Location, System.Drawing.Point.Empty, rectangle.Size); | |
| } | |
| // 3. Save to temp file | |
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | |
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | |
| return GingerOcrOperations.ReadTextFromImage(tempImagePath); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex); | |
| } | |
| finally | |
| { | |
| // 5. Clean up temp file | |
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | |
| { | |
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | |
| } | |
| } | |
| // fallback: use normal value getter | |
| return GetControlValue(element); | |
| } | |
| protected virtual string GetControlValueByOCR(UIAuto.AutomationElement element) | |
| { | |
| string tempImagePath = null; | |
| try | |
| { | |
| if (element == null) return string.Empty; | |
| // 1) Bring the window to the foreground and force a paint | |
| try | |
| { | |
| var status = WinAPIAutomation.SetForeGroundWindow(CurrentWindow.Current.ProcessId); | |
| if (!status) { WinAPIAutomation.ShowWindow(CurrentWindow); } | |
| ForcePaint(CurrentWindow.Current.NativeWindowHandle); | |
| } | |
| catch { /* best-effort */ } | |
| var er = element.Current.BoundingRectangle; | |
| if (er.Width <= 0 || er.Height <= 0) return GetControlValue(element); | |
| // Prefer occlusion-safe window capture, then crop to element bounds | |
| using (var winBmp = WindowToBitmap(CurrentWindow)) | |
| { | |
| if (winBmp != null) | |
| { | |
| var wr = CurrentWindow.Current.BoundingRectangle; | |
| var crop = new System.Drawing.Rectangle( | |
| er.X - wr.X, | |
| er.Y - wr.Y, | |
| er.Width, | |
| er.Height | |
| ); | |
| crop = System.Drawing.Rectangle.Intersect( | |
| crop, | |
| new System.Drawing.Rectangle(0, 0, winBmp.Width, winBmp.Height) | |
| ); | |
| if (crop.Width > 0 && crop.Height > 0) | |
| { | |
| using (var bmp = winBmp.Clone(crop, winBmp.PixelFormat)) | |
| { | |
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | |
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | |
| return GingerOcrOperations.ReadTextFromImage(tempImagePath)?.Trim(); | |
| } | |
| } | |
| } | |
| } | |
| // 2) Fallback: CopyFromScreen with clipping to virtual screen | |
| var vs = System.Windows.Forms.SystemInformation.VirtualScreen; | |
| var screenRect = System.Drawing.Rectangle.Intersect( | |
| new System.Drawing.Rectangle(er.X, er.Y, er.Width, er.Height), | |
| new System.Drawing.Rectangle(vs.X, vs.Y, vs.Width, vs.Height) | |
| ); | |
| if (screenRect.Width <= 0 || screenRect.Height <= 0) return GetControlValue(element); | |
| using (var bmp = new Bitmap(screenRect.Width, screenRect.Height)) | |
| using (var g = Graphics.FromImage(bmp)) | |
| { | |
| g.CopyFromScreen( | |
| new System.Drawing.Point(screenRect.X, screenRect.Y), | |
| System.Drawing.Point.Empty, | |
| screenRect.Size | |
| ); | |
| tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); | |
| bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); | |
| return GingerOcrOperations.ReadTextFromImage(tempImagePath)?.Trim(); | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.DEBUG, "GetValueByOCR fallback to GetControlValue", ex); | |
| } | |
| finally | |
| { | |
| if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) | |
| { | |
| try { File.Delete(tempImagePath); } catch { /* ignore */ } | |
| } | |
| } | |
| return GetControlValue(element); | |
| } |
Comment on lines
+923
to
+952
| /// <summary> | ||
| /// Brings the window containing the given AutomationElement to the foreground and optionally resizes it. | ||
| /// </summary> | ||
| /// <param name="automationElement">The UI Automation element whose window should be focused.</param> | ||
| /// <param name="resizeWidth">Optional width to resize the window to.</param> | ||
| /// <param name="resizeHeight">Optional height to resize the window to.</param> | ||
| public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null) | ||
| { | ||
| if (automationElement == null) | ||
| return; | ||
|
|
||
| try | ||
| { | ||
| // Bring window to foreground | ||
| WinAPIAutomation.ShowWindow(automationElement); | ||
|
|
||
| // Optionally resize | ||
| if (resizeWidth.HasValue && resizeHeight.HasValue) | ||
| { | ||
| WinAPIAutomation.ResizeExternalWindow(automationElement, resizeWidth.Value, resizeHeight.Value); | ||
| } | ||
|
|
||
| // Give time to the window to come to the front | ||
| Thread.Sleep(500); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Reporter.ToLog(eLogLevel.ERROR, "Failed to bring window to foreground", ex); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Resolve top-level window before ShowWindow/Resize; reduce foreground flakiness.
ShowWindow on a child can be a no-op. Bubble to the ancestor ControlType.Window, then act on it. Optionally replace the fixed sleep with a short poll if a foreground-check API exists.
- public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null)
+ public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null)
{
if (automationElement == null)
return;
try
{
- // Bring window to foreground
- WinAPIAutomation.ShowWindow(automationElement);
+ // Resolve top-level window
+ var topWindow = automationElement;
+ while (topWindow != null && topWindow.Current.ControlType != UIAuto.ControlType.Window)
+ {
+ topWindow = UIAuto.TreeWalker.RawViewWalker.GetParent(topWindow);
+ }
+ var target = topWindow ?? automationElement;
+
+ // Bring window to foreground
+ WinAPIAutomation.ShowWindow(target);
// Optionally resize
if (resizeWidth.HasValue && resizeHeight.HasValue)
{
- WinAPIAutomation.ResizeExternalWindow(automationElement, resizeWidth.Value, resizeHeight.Value);
+ WinAPIAutomation.ResizeExternalWindow(target, resizeWidth.Value, resizeHeight.Value);
}
- // Give time to the window to come to the front
- Thread.Sleep(500);
+ // Short settle wait (consider polling for foreground if API available)
+ Thread.Sleep(200);
}
catch (Exception ex)
{
Reporter.ToLog(eLogLevel.ERROR, "Failed to bring window to foreground", ex);
}
}To switch from fixed sleep to polling, confirm if WinAPIAutomation exposes a foreground query:
🏁 Script executed:
#!/bin/bash
# Look for foreground helpers we can poll on
rg -nP -C2 '\b(Get|Set)ForegroundWindow\b|IsForeground|GetActiveWindow|SetActiveWindow|WindowFromPoint' Ginger | sed -n '1,200p'Length of output: 17290
🏁 Script executed:
#!/bin/bash
# Search for BringElementWindowToForeground definition and its file location
rg -n 'BringElementWindowToForeground' -C2 -g '*.cs'Length of output: 1315
Bubble to top‐level Window before ShowWindow/Resize and poll for foreground
- In UIElementOperationsHelper.cs BringElementWindowToForeground, walk up the UI tree via RawViewWalker.GetParent until Current.ControlType == ControlType.Window and use that element for WinAPIAutomation.ShowWindow/ResizeExternalWindow.
- Replace Thread.Sleep(500) with a short polling loop against WinAPIAutomation.GetForegroundWindow (fallback to ~200 ms delay if needed).
🤖 Prompt for AI Agents
In Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs around lines
923 to 952, the method should first bubble the supplied AutomationElement to its
top-level Window: use UIAuto.RawViewWalker.GetParent in a loop until
element.Current.ControlType == UIAuto.ControlType.Window (or no parent) and use
that window element for WinAPIAutomation.ShowWindow and ResizeExternalWindow.
Remove the fixed Thread.Sleep(500) and instead poll
WinAPIAutomation.GetForegroundWindow in a short loop (e.g., check every 20-50ms)
until it equals the target window handle or a timeout (~200ms) elapses; if
resize is requested call ResizeExternalWindow on the window element before/after
ShowWindow as needed, and only fall back to a final small Thread.Sleep if
GetForegroundWindow is unavailable. Ensure exceptions are still logged as
before.
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 (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
9672-9699: Handle GetValueByOCR explicitly to avoid misleading behavior on WebSelenium currently lacks OCR. Add an explicit fallback (with INFO log) rather than silently duplicating GetValue semantics elsewhere.
Apply:
switch (act.ElementAction) { + case ActUIElement.eElementAction.GetValueByOCR: + Reporter.ToLog(eLogLevel.INFO, "GetValueByOCR is not supported by Selenium; falling back to DOM value (GetValue)."); + goto case ActUIElement.eElementAction.GetValue; + case ActUIElement.eElementAction.GetValue: if (act.ElementType == eElementType.HyperLink) {Alternative: mark as Skipped if you prefer strict capability signaling.
+ case ActUIElement.eElementAction.GetValueByOCR: + act.Status = Amdocs.Ginger.CoreNET.Execution.eRunStatus.Skipped; + act.ExInfo = "GetValueByOCR is not supported by Selenium driver."; + break;Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (1)
4105-4136: Make the STA clipboard fallback thread-safe and bounded (avoid spin-wait + visibility issues).Issues:
- bDone is not volatile; the write may not become visible on the reader thread promptly.
- Busy-wait loop without timeout risks hangs if the STA thread stalls.
- Thread isn’t marked as background; it can keep the process alive.
- Prefer UnicodeText over Text for reliability.
Suggested fix with ManualResetEventSlim, timeout, background thread, and UnicodeText:
private static string GetClipboardText() { string clipboardText = GingerCore.General.GetClipboardText(); - - if (string.IsNullOrEmpty(clipboardText)) + if (string.IsNullOrEmpty(clipboardText)) { Reporter.ToLog(eLogLevel.DEBUG, "Clipboard couldn't be identified, so trying alternative."); - - bool bDone = false; - // Fall-back mechanism, if above GingerCore.General.GetClipboardText doesn't work, then - var t = new Thread(() => - { - try - { - clipboardText = Clipboard.GetText(TextDataFormat.Text); - bDone = true; - } - catch (Exception ex) - { - Reporter.ToLog(eLogLevel.DEBUG, "Exception in GetClipboardText", ex); - bDone = true; - } - }); - - t.SetApartmentState(ApartmentState.STA); - t.Start(); - while (bDone == false) - { - Thread.Sleep(100); - } + // Fall-back mechanism, if GingerCore.General.GetClipboardText() doesn't work. + var mre = new ManualResetEventSlim(false); + string staText = null; + Exception staEx = null; + var t = new Thread(() => + { + try + { + if (Clipboard.ContainsText(TextDataFormat.UnicodeText)) + staText = Clipboard.GetText(TextDataFormat.UnicodeText); + else if (Clipboard.ContainsText(TextDataFormat.Text)) + staText = Clipboard.GetText(TextDataFormat.Text); + } + catch (Exception ex) + { + staEx = ex; + } + finally + { + mre.Set(); + } + }); + t.IsBackground = true; + t.SetApartmentState(ApartmentState.STA); + t.Start(); + if (mre.Wait(TimeSpan.FromSeconds(2))) + { + if (!string.IsNullOrEmpty(staText)) + { + clipboardText = staText; + } + else if (staEx != null) + { + Reporter.ToLog(eLogLevel.DEBUG, "Exception in GetClipboardText", staEx); + } + } + else + { + Reporter.ToLog(eLogLevel.DEBUG, "GetClipboardText STA fallback timed out."); + } } return clipboardText; }
♻️ Duplicate comments (4)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
4650-4651: Fix null-conditional chain that can throw when Locators is nullCalling Where on a null sequence still throws. Coalesce Locators before LINQ.
Apply:
- Reporter.ToLog(eLogLevel.ERROR, $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", currentPOMElementInfo?.Locators?.Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'") ?? Enumerable.Empty<string>())}]", ex); + Reporter.ToLog( + eLogLevel.ERROR, + $"Failed to locate element '{currentPOMElementInfo?.ElementName}' ActiveLocators=[{string.Join(", ", (currentPOMElementInfo?.Locators ?? Enumerable.Empty<ElementLocator>()).Where(l => l.Active).Select(l => $"{l.LocateBy}='{l.LocateValue}'"))}]", + ex);Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (3)
22-27: Add OS guard import for System.Drawing usage.System.Drawing is Windows-only on .NET 6+. Add System.Runtime.InteropServices (and guard in method below).
using System.Diagnostics; using System.Drawing; using System.IO; +using System.Runtime.InteropServices;
937-961: Use top-level window for foregrounding to reduce flakiness.ShowWindow on a child element can be a no-op. Resolve ancestor ControlType.Window, then call ShowWindow/Resize on it.
- public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null) + public static void BringElementWindowToForeground(UIAuto.AutomationElement automationElement, int? resizeWidth = null, int? resizeHeight = null) { if (automationElement == null) { return; } try { - // Bring window to foreground - WinAPIAutomation.ShowWindow(automationElement); + // Resolve top-level window + var topWindow = automationElement; + while (topWindow != null && topWindow.Current.ControlType != UIAuto.ControlType.Window) + { + topWindow = UIAuto.TreeWalker.RawViewWalker.GetParent(topWindow); + } + var target = topWindow ?? automationElement; + // Bring window to foreground + WinAPIAutomation.ShowWindow(target); // Optionally resize if (resizeWidth.HasValue && resizeHeight.HasValue) { - WinAPIAutomation.ResizeExternalWindow(automationElement, resizeWidth.Value, resizeHeight.Value); + WinAPIAutomation.ResizeExternalWindow(target, resizeWidth.Value, resizeHeight.Value); } // Give time to the window to come to the front Thread.Sleep(500); } catch (Exception ex) { Reporter.ToLog(eLogLevel.ERROR, "Failed to bring window to foreground", ex); } }
574-631: Fix compile break and add Windows/NULL guards in GetValueByOCR.BoundingRectangle is a System.Windows.Rect; assigning to System.Drawing.Rectangle won’t compile. Also guard for non-Windows and null element to avoid runtime errors.
- public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) + public static ActionResult GetValueByOCR(UIAuto.AutomationElement automationElement) { ActionResult actionResult = new ActionResult(); string tempImagePath = null; - try + // Guards + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + actionResult.errorMessage = "GetValueByOCR is supported on Windows only."; + return actionResult; + } + if (automationElement == null) + { + actionResult.errorMessage = "Automation element is null."; + return actionResult; + } + try { BringElementWindowToForeground(automationElement); // 1. Get bounding rectangle - Rectangle rect = automationElement.Current.BoundingRectangle; - - int left = rect.Left; - int top = rect.Top; - int width = rect.Width; - int height = rect.Height; + var rect = automationElement.Current.BoundingRectangle; // System.Windows.Rect + int left = (int)rect.Left; + int top = (int)rect.Top; + int width = (int)rect.Width; + int height = (int)rect.Height; if (width <= 0 || height <= 0) { actionResult.errorMessage = "Element has zero-sized bounding rectangle."; return actionResult; } // 2. Capture screenshot using (var bmp = new Bitmap(width, height)) { using (var g = Graphics.FromImage(bmp)) { - g.CopyFromScreen(left, top, 0, 0, new Size(width, height)); + g.CopyFromScreen(left, top, 0, 0, new Size(width, height)); } // 3. Save to temp file tempImagePath = Path.Combine(Path.GetTempPath(), $"GingerOCR_{Guid.NewGuid()}.png"); bmp.Save(tempImagePath, System.Drawing.Imaging.ImageFormat.Png); } // 4. OCR string ocrText = GingerOcrOperations.ReadTextFromImage(tempImagePath); - actionResult.outputValue = string.IsNullOrWhiteSpace(ocrText) ? ocrText : ocrText.Trim(); + actionResult.outputValue = string.IsNullOrWhiteSpace(ocrText) ? ocrText : ocrText.Trim(); actionResult.executionInfo = "OCR value extracted successfully"; } catch (Exception ex) { actionResult.errorMessage = "Failed to get value by OCR: " + ex.Message; } finally { // 5. Clean up temp file if (!string.IsNullOrEmpty(tempImagePath) && File.Exists(tempImagePath)) { try { File.Delete(tempImagePath); } catch (Exception ex) { Reporter.ToLog(eLogLevel.ERROR, $"Unable to delete the error file error : {ex}"); } } } return actionResult; }
📜 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 (5)
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs(3 hunks)Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs(4 hunks)Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs(2 hunks)Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (30)
📚 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/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.csGinger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.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.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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.csGinger/GingerCore/Drivers/Common/UIElementOperationsHelper.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-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-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-08-22T16:31:58.569Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:171-174
Timestamp: 2025-08-22T16:31:58.569Z
Learning: In GingerCoreNET, the canonical FilterProperties list lives in Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/POM/POMUtils.cs and should not be duplicated in other classes (e.g., SeleniumDriver.cs).
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-09-05T14:06:10.282Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4296
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5693-5693
Timestamp: 2025-09-05T14:06:10.282Z
Learning: Project baseline is .NET 8 / C# 12+. Prefer collection expressions `[]` for initializing collections (List<T>, Dictionary<TKey,TValue>, Stack<T>, etc.) across the codebase, including SeleniumDriver.cs around the POM learning section; do not suggest replacing `[]` with `new()` or `new List<T>()`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-11-06T09:06:34.050Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#3984
File: Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs:911-911
Timestamp: 2024-11-06T09:06:34.050Z
Learning: In the `Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs` file of the Ginger project, initializing lists in C# using `[]` syntax (e.g., `List<T> list = [];`) is acceptable and does not require changing to `new List<T>()`.
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-09-30T18:11:57.769Z
Learnt from: Maheshkale447
PR: Ginger-Automation/Ginger#3945
File: Ginger/GingerCoreNET/Repository/SharedRepositoryOperations.cs:550-550
Timestamp: 2024-09-30T18:11:57.769Z
Learning: In C# with .NET 8 and later, it's valid to initialize lists using the `[]` syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-16T04:08:12.351Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#4058
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:652-653
Timestamp: 2025-01-16T04:08:12.351Z
Learning: C# 12 introduced collection expressions that allow initializing arrays and other collection types using a simplified syntax with square brackets. For example, `[item1, item2]` is equivalent to `new[] { item1, item2 }`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-01-16T04:09:15.662Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#4058
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:648-649
Timestamp: 2025-01-16T04:09:15.662Z
Learning: C# 12 supports collection expressions with spread operator, allowing syntax like `[item1, .. collection]` for creating arrays and other collections.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-03-28T13:45:52.031Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4149
File: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/AgentConfigOperations.cs:40-40
Timestamp: 2025-03-28T13:45:52.031Z
Learning: The empty collection initializer syntax `[]` is valid in C# 12 and newer versions. For example, `List<T> list = []` is equivalent to `List<T> list = new List<T>()` in C# 12+.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-10-28T11:12:49.263Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3971
File: Ginger/GingerCoreNET/External/Katalon/Conversion/KatalonObjectRepositoryToPOMConverter.cs:37-37
Timestamp: 2024-10-28T11:12:49.263Z
Learning: In the Ginger project, initializing collections in C# using `[]` is valid syntax.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-08-25T08:48:18.970Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml.cs:51-63
Timestamp: 2025-08-25T08:48:18.970Z
Learning: In the Ginger project, `[]` collection expression syntax is the preferred modern approach for initializing collections, including dictionaries (e.g., `mAct.AlertItems = [];`), as confirmed by the project maintainers.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2024-10-28T13:01:46.614Z
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#3971
File: Ginger/GingerCoreNET/External/Katalon/Conversion/KatalonElementToElementInfoConverter.cs:27-27
Timestamp: 2024-10-28T13:01:46.614Z
Learning: In `KatalonElementToElementInfoConverter.cs`, initializing a `List<ElementInfo>` using `List<ElementInfo> elementInfoList = [];` is acceptable in the project.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs
📚 Learning: 2025-04-08T12:31:45.924Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4169
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:321-336
Timestamp: 2025-04-08T12:31:45.924Z
Learning: The GetTextLengthAsync method in Ginger Automation's Web driver should attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue) to calculate text length, providing enhanced capabilities to read text length from different types of elements.
Applied to files:
Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs
📚 Learning: 2025-03-20T11:10:33.780Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:33.780Z
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/GingerCore/Drivers/Common/UIElementOperationsHelper.csGinger/GingerCoreCommon/Actions/UI Element/ActUIElement.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/GingerCoreCommon/Actions/UI Element/ActUIElement.cs
📚 Learning: 2025-06-13T13:47:39.193Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4228
File: Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs:384-394
Timestamp: 2025-06-13T13:47:39.193Z
Learning: For the `eUnlockTypes` enum in `Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs`, the identifiers must remain lowercase (`pin`, `password`, `pattern`, `none`) per project requirements.
Applied to files:
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.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/Actions/UI Element/ActUIElement.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/GingerCoreCommon/Actions/UI Element/ActUIElement.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/GingerCoreCommon/Actions/UI Element/ActUIElement.cs
🔇 Additional comments (10)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (3)
171-171: No-op formatting changeWhitespace-only; nothing to review.
5693-5693: LGTM: modern collection expressionUsing [] aligns with the .NET 8/C# 12 project baseline and team preference.
5851-5851: LGTM: ensure batched updates are awaitedAwaiting UpdateAndMarkElementsAsync avoids dropped work when flushing.
Ginger/GingerCore/Actions/UIAutomation/UIAComWrapperHelper.cs (2)
2438-2444: No functional change here; safe to ignore.
5299-5310: Row match (Equals) loop looks correct.Sequential scan with GetControlValueForComparision is consistent with other operators and short-circuits on first match. No issues spotted.
Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs (2)
27-29: LGTM: using directives reorder is fine.
221-534: No numeric enum persistence detected. All eElementAction serializations use names via Enum.Parse/string APIs; no int casts for storage were found.Ginger/GingerCore/Drivers/WindowsLib/WindowsDriver.cs (2)
452-455: Correct: mark OCR value as output.Setting isoutputvalue = true ensures OCR text is returned consistently. Good fix.
1175-1176: LGTM: minor predicate formatting only.Ginger/GingerCore/Drivers/Common/UIElementOperationsHelper.cs (1)
874-893: LGTM: robust OCR validation path.Early null-break, trim compare, and GetText fallback are correct.
Comment on lines
+461
to
+462
| [EnumValueDescription("Get Value By OCR")] | ||
| GetValueByOCR, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expose OCR result in the Outputs grid.
Add GetValueByOCR to GetDisplayOutputValue so users see the extracted text in the Outputs grid like GetValue/GetText.
- return ElementAction switch
- {
- eElementAction.Click or eElementAction.Collapse or eElementAction.Expand => false,
- eElementAction.GetValue => true,
- _ => false,
- };
+ return ElementAction switch
+ {
+ eElementAction.Click or eElementAction.Collapse or eElementAction.Expand => false,
+ eElementAction.GetValue or eElementAction.GetValueByOCR => true,
+ _ => false,
+ };📝 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
| [EnumValueDescription("Get Value By OCR")] | |
| GetValueByOCR, | |
| // In ActUIElement.cs, update the GetDisplayOutputValue (or equivalent) method: | |
| return ElementAction switch | |
| { | |
| eElementAction.Click or | |
| eElementAction.Collapse or | |
| eElementAction.Expand => false, | |
| eElementAction.GetValue or | |
| eElementAction.GetValueByOCR => true, | |
| _ => false, | |
| }; |
🤖 Prompt for AI Agents
In Ginger/GingerCoreCommon/Actions/UI Element/ActUIElement.cs around lines
461-462, the new enum member GetValueByOCR was added but not handled in
GetDisplayOutputValue; update that method to treat GetValueByOCR the same as
GetValue/GetText by adding a case for GetValueByOCR that returns the
OCR-extracted text (the same property used by GetValue/GetText output—e.g., the
action's output Data/Text property or the OCR-specific output field) so the
extracted OCR string appears in the Outputs grid.
Maheshkale447
approved these changes
Sep 5, 2025
15 tasks
This was referenced Sep 18, 2025
Merged
Merged
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
Style