You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
WebDriverListener has now default methods for both WebDriver and WebElement for clients to be notified before and after a screenshot is being taken.
🔄 Types of changes
New feature (non-breaking change which adds functionality and tests!)
PR Type
Enhancement
Description
Add screenshot hooks to WebDriverListener interface
Support both WebDriver and WebElement screenshot events
Include before/after methods with target and result parameters
Add comprehensive test coverage for new functionality
Diagram Walkthrough
flowchart LR
A["WebDriverListener Interface"] --> B["Add beforeGetScreenshotAs methods"]
A --> C["Add afterGetScreenshotAs methods"]
B --> D["WebDriver screenshot hooks"]
B --> E["WebElement screenshot hooks"]
C --> D
C --> E
F["Test Coverage"] --> G["Mock screenshot calls"]
F --> H["Verify event firing order"]
The Javadoc for WebElement's afterGetScreenshotAs references TargetLocator#alert(), which appears to be a copy/paste error; it should reference WebElement#getScreenshotAs.
/** * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is * called. * * @param element - decorated WebElement instance * @param target - target type, @see OutputType * @param result - object in which is stored information about the screenshot. * @param <X> - return type for getScreenshotAs. */default <X> voidafterGetScreenshotAs(WebElementelement, OutputType<X> target, Xresult) {}
Consider bounding the generic parameter to align with OutputType usage (e.g., is fine, but verify consistency with existing listener generics and any serialization constraints).
default <X> voidbeforeGetScreenshotAs(WebDriverdriver, OutputType<X> target) {}
/** * This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called. * * @param driver - decorated WebDriver instance * @param target - target type, @see OutputType * @param result - object in which is stored information about the screenshot. * @param <X> - return type for getScreenshotAs. */default <X> voidafterGetScreenshotAs(WebDriverdriver, OutputType<X> target, Xresult) {}
The new WebElement screenshot hooks have misleading Javadoc: afterGetScreenshotAs references TargetLocator.alert(), and both before/after methods for WebDriver/WebElement should clearly state they correspond to getScreenshotAs on TakesScreenshot/WebElement. Please correct the Javadoc and ensure consistency of method naming and generics in docs to avoid API confusion for users implementing listeners.
/** * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is * called. * * @param element - decorated WebElement instance * @param target - target type, @see OutputType * @param result - object in which is stored information about the screenshot. * @param <X> - return type for getScreenshotAs. */default <X> voidafterGetScreenshotAs(WebElementelement, OutputType<X> target, Xresult) {}
/** * @param element - decorated WebElement instance * @param target - target type, @see OutputType * @param <X> - return type for getScreenshotAs. */default <X> voidbeforeGetScreenshotAs(WebElementelement, OutputType<X> target) {}
Solution Walkthrough:
Before:
/** * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is * called. * ... */default <X> voidafterGetScreenshotAs(WebElementelement, OutputType<X> target, Xresult) {}
/** * @param element - decorated WebElement instance * ... */default <X> voidbeforeGetScreenshotAs(WebElementelement, OutputType<X> target) {}
After:
/** * This action will be performed each time after {@link WebElement#getScreenshotAs(OutputType)} is * called. * ... */default <X> voidafterGetScreenshotAs(WebElementelement, OutputType<X> target, Xresult) {}
/** * This method will be called before {@link WebElement#getScreenshotAs(OutputType)} is called. * @param element - decorated WebElement instance * ... */default <X> voidbeforeGetScreenshotAs(WebElementelement, OutputType<X> target) {}
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies a significant copy-paste error in the Javadoc for afterGetScreenshotAs and incompleteness in beforeGetScreenshotAs, which are crucial for API contract clarity.
Medium
Learned best practice
✅ Fix incorrect Javadoc referenceSuggestion Impact:The commit updated the Javadoc to remove the incorrect TargetLocator#alert() reference and replaced it with a getScreenshotAs reference, aligning with the suggestion’s intent to fix the description. It also added a corresponding "before" Javadoc. However, it referenced TakesScreenshot instead of WebElement and did not adjust parameter descriptions/punctuation as suggested.
code diff:
- * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is- * called.+ * This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
* @param element - decorated WebElement instance
* @param target - target type, @see OutputType
The Javadoc text references TargetLocator#alert(), which is unrelated to element screenshots, and misuses "@see" in a @param. Update the description to reflect WebElement.getScreenshotAs and fix parameter docs and punctuation.
/**
- * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is- * called.+ * This method will be called after {@link WebElement#getScreenshotAs(OutputType)} is called.
*
- * @param element - decorated WebElement instance- * @param target - target type, @see OutputType- * @param result - object in which is stored information about the screenshot.- * @param <X> - return type for getScreenshotAs.+ * @param element the decorated WebElement instance+ * @param target the target type, {@link OutputType}+ * @param result the screenshot result+ * @param <X> the return type for getScreenshotAs
*/
default <X> void afterGetScreenshotAs(WebElement element, OutputType<X> target, X result) {}
[Suggestion processed]
Suggestion importance[1-10]: 6
__
Why:
Relevant best practice - Fix syntax errors, typos, and naming inconsistencies in comments and documentation.
Low
✅ Correct Javadoc parameter linksSuggestion Impact:The commit updated Javadoc around getScreenshotAs, adding/improving method description lines similarly to the suggestion’s intent. However, it did not convert the @param tags to use {@link OutputType}; those still use "@see". So the impact is partial and indirect.
code diff:
+ * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is+ * called.+ *
* @param element - decorated WebElement instance
* @param target - target type, @see OutputType
* @param <X> - return type for getScreenshotAs.
@@ -611,8 +614,7 @@
default <X> void beforeGetScreenshotAs(WebElement element, OutputType<X> target) {}
/**
- * This action will be performed each time after {@link WebDriver.TargetLocator#alert()} is- * called.+ * This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
* @param element - decorated WebElement instance
* @param target - target type, @see OutputType
In Javadoc, avoid using "@see" within @param descriptions; link the type directly with {@link}. This improves clarity and tooling parsing. Update the parameter docs to use proper links and consistent punctuation.
/**
- * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is- * called.+ * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param driver - decorated WebDriver instance- * @param target - target type, @see OutputType- * @param <X> - return type for getScreenshotAs.+ * @param driver the decorated WebDriver instance+ * @param target the target type, {@link OutputType}+ * @param <X> the return type for getScreenshotAs
*/
default <X> void beforeGetScreenshotAs(WebDriver driver, OutputType<X> target) {}
[Suggestion processed]
Suggestion importance[1-10]: 5
__
Why:
Relevant best practice - Fix string formatting and interpolation issues; avoid ambiguous or incorrect Javadoc tags and references.
Low
General
Tighten generic bound for type safety
Constrain the generic to match Selenium's OutputType contract to avoid raw/unsafe usage. Add an extends Object bound to make it explicit and consistent with similar APIs in the codebase.
/**
* This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is
* called.
*
* @param driver - decorated WebDriver instance
* @param target - target type, @see OutputType
* @param <X> - return type for getScreenshotAs.
*/
-default <X> void beforeGetScreenshotAs(WebDriver driver, OutputType<X> target) {}+default <X extends Object> void beforeGetScreenshotAs(WebDriver driver, OutputType<X> target) {}
Apply / Chat
Suggestion importance[1-10]: 3
__
Why: The suggestion to add <X extends Object> is a stylistic improvement for consistency, as <X> is already implicitly bounded by Object in Java, so this change has no functional impact.
Low
Align generic bound for consistency
Keep the generic parameter consistent and explicitly bounded to avoid erasure ambiguities and maintain type safety with OutputType. Mirror the bound used in the corresponding "before" hook.
/**
* This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
* @param driver - decorated WebDriver instance
* @param target - target type, @see OutputType
* @param result - object in which is stored information about the screenshot.
* @param <X> - return type for getScreenshotAs.
*/
-default <X> void afterGetScreenshotAs(WebDriver driver, OutputType<X> target, X result) {}+default <X extends Object> void afterGetScreenshotAs(WebDriver driver, OutputType<X> target, X result) {}
Apply / Chat
Suggestion importance[1-10]: 3
__
Why: The suggestion to add <X extends Object> is a stylistic improvement for consistency, as <X> is already implicitly bounded by Object in Java, so this change has no functional impact.
The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.
We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us
The Javadoc uses '@see' in parameter descriptions (e.g., 'target - target type, @see OutputType'); parameter docs should avoid tags or use 'see also' section. Also consider clarifying generic type bounds for 'X' to mirror 'OutputType' usage.
/** * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is * called. * * @param driver - decorated WebDriver instance * @param target - target type, @see OutputType * @param <X> - return type for getScreenshotAs. */default <X> voidbeforeGetScreenshotAs(WebDriverdriver, OutputType<X> target) {}
/** * This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called. * * @param driver - decorated WebDriver instance * @param target - target type, @see OutputType * @param result - object in which is stored information about the screenshot. * @param <X> - return type for getScreenshotAs. */default <X> voidafterGetScreenshotAs(WebDriverdriver, OutputType<X> target, Xresult) {}
In the anonymous listener for WebElement screenshots, the parameter is named 'driver' instead of 'element' in before/after methods; align naming for readability and to avoid confusion in future refactors.
✅ Clean up Javadoc referencesSuggestion Impact:The commit updated the Javadoc @param lines to remove dashes, replace "@see OutputType" with "see {@link OutputType}", clarified the result description, and removed trailing periods, matching the suggested cleanup.
code diff:
- * @param driver - decorated WebDriver instance- * @param target - target type, @see OutputType- * @param <X> - return type for getScreenshotAs.+ * @param driver decorated WebDriver instance+ * @param target target type, see {@link OutputType}+ * @param <X> return type for getScreenshotAs
*/
default <X> void beforeGetScreenshotAs(WebDriver driver, OutputType<X> target) {}
/**
* This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param driver - decorated WebDriver instance- * @param target - target type, @see OutputType- * @param result - object in which is stored information about the screenshot.- * @param <X> - return type for getScreenshotAs.+ * @param driver decorated WebDriver instance+ * @param target target type, see {@link OutputType}+ * @param result object that stores the screenshot information+ * @param <X> return type for getScreenshotAs
*/
default <X> void afterGetScreenshotAs(WebDriver driver, OutputType<X> target, X result) {}
@@ -607,19 +607,19 @@
* This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is
* called.
*
- * @param element - decorated WebElement instance- * @param target - target type, @see OutputType- * @param <X> - return type for getScreenshotAs.+ * @param element decorated WebElement instance+ * @param target target type, see {@link OutputType}+ * @param <X> return type for getScreenshotAs
*/
default <X> void beforeGetScreenshotAs(WebElement element, OutputType<X> target) {}
/**
* This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param element - decorated WebElement instance- * @param target - target type, @see OutputType- * @param result - object in which is stored information about the screenshot.- * @param <X> - return type for getScreenshotAs.+ * @param element decorated WebElement instance+ * @param target target type, see {@link OutputType}+ * @param result result object that stores the screenshot information+ * @param <X> return type for getScreenshotAs
Replace "@see" inline in @param descriptions with proper Javadoc links or move them to @see tags. Also clarify generic return type wording and remove trailing periods in parameter descriptions for consistency.
/**
- * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is- * called.+ * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param driver - decorated WebDriver instance- * @param target - target type, @see OutputType- * @param <X> - return type for getScreenshotAs.+ * @param driver decorated WebDriver instance+ * @param target target type, see {@link OutputType}+ * @param <X> return type for getScreenshotAs
*/
default <X> void beforeGetScreenshotAs(WebDriver driver, OutputType<X> target) {}
/**
* This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param driver - decorated WebDriver instance- * @param target - target type, @see OutputType- * @param result - object in which is stored information about the screenshot.- * @param <X> - return type for getScreenshotAs.+ * @param driver decorated WebDriver instance+ * @param target target type, see {@link OutputType}+ * @param result object that stores the screenshot information+ * @param <X> return type for getScreenshotAs
*/
default <X> void afterGetScreenshotAs(WebDriver driver, OutputType<X> target, X result) {}
/**
- * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is- * called.+ * This method will be called before {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param element - decorated WebElement instance- * @param target - target type, @see OutputType- * @param <X> - return type for getScreenshotAs.+ * @param element decorated WebElement instance+ * @param target target type, see {@link OutputType}+ * @param <X> return type for getScreenshotAs
*/
default <X> void beforeGetScreenshotAs(WebElement element, OutputType<X> target) {}
/**
* This method will be called after {@link TakesScreenshot#getScreenshotAs(OutputType)} is called.
*
- * @param element - decorated WebElement instance- * @param target - target type, @see OutputType- * @param result - object in which is stored information about the screenshot.- * @param <X> - return type for getScreenshotAs.+ * @param element decorated WebElement instance+ * @param target target type, see {@link OutputType}+ * @param result object that stores the screenshot information+ * @param <X> return type for getScreenshotAs
*/
default <X> void afterGetScreenshotAs(WebElement element, OutputType<X> target, X result) {}
[Suggestion processed]
Suggestion importance[1-10]: 6
__
Why:
Relevant best practice - Fix Javadoc formatting and reference style to avoid ambiguity and maintain consistent documentation quality.
Low
General
✅ Fix misleading parameter nameSuggestion Impact:The commit renames the parameter from 'driver' to 'element' in both beforeGetScreenshotAs and afterGetScreenshotAs methods as suggested.
code diff:
- public <X> void beforeGetScreenshotAs(WebElement driver, OutputType<X> target) {+ public <X> void beforeGetScreenshotAs(WebElement element, OutputType<X> target) {
acc.append("beforeGetScreenshotAs ").append(target).append("\n");
}
@Override
- public <X> void afterGetScreenshotAs(WebElement driver, OutputType<X> target, X result) {+ public <X> void afterGetScreenshotAs(WebElement element, OutputType<X> target, X result) {
The parameter name driver for WebElement callbacks is misleading and contradicts the interface, risking confusion and incorrect usage. Rename it to element to align with WebDriverListener method signatures and improve test clarity.
Why: The suggestion correctly identifies a misleading parameter name driver for a WebElement and suggests renaming it to element for consistency with the interface, improving code clarity.
…HQ#16232
WebDriverListener has now default methods for both WebDriver and
WebElement for clients to be notified before and after a
screenshot is being taken.
FixesSeleniumHQ#16232
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.
selenium-ci
added
C-java
giulong
force-pushed
the
feature/issue-16232
branch
from
User description
🔗 Related Issues
Fixes #16232
💥 What does this PR do?
WebDriverListenerhas now default methods for bothWebDriverandWebElementfor clients to be notified before and after a screenshot is being taken.🔄 Types of changes
PR Type
Enhancement
Description
Add screenshot hooks to
WebDriverListenerinterfaceSupport both
WebDriverandWebElementscreenshot eventsInclude before/after methods with target and result parameters
Add comprehensive test coverage for new functionality
Diagram Walkthrough
File Walkthrough
WebDriverListener.java
Add screenshot event hooks to listener interfacejava/src/org/openqa/selenium/support/events/WebDriverListener.java
OutputTypeandTakesScreenshotimportsbeforeGetScreenshotAsandafterGetScreenshotAsmethods forWebDriverbeforeGetScreenshotAsandafterGetScreenshotAsmethods forWebElementEventFiringDecoratorTest.java
Add comprehensive tests for screenshot hooksjava/test/org/openqa/selenium/support/events/EventFiringDecoratorTest.java
OutputTypeandTakesScreenshotWebDriverscreenshot events