bito-code-review[bot] commented on code in PR #36027:
URL: https://github.com/apache/superset/pull/36027#discussion_r2499893399
##########
superset/utils/screenshot_utils.py:
##########
@@ -156,12 +156,28 @@ def take_tiled_screenshot(
remaining_content = dashboard_height - tile_start_in_element
tile_content_height = min(viewport_height, remaining_content)
+ # Determine clip height (use visible element height in viewport)
+ clip_height = min(tile_content_height,
current_element_box["height"])
+
+ # Skip tile if dimensions are invalid (width or height <= 0)
+ # This can happen if element is completely scrolled out of viewport
+ if clip_height <= 0:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete clip dimension validation</b></div>
<div id="fix">
The validation added to skip tiles with invalid clip dimensions only checks
if `clip_height <= 0`, but Playwright's `page.screenshot()` requires both width
and height to be greater than zero. If `current_element_box["width"]` is less
than or equal to zero, the code will proceed and attempt to take a screenshot
with an invalid clip, causing a failure. This impacts the
`take_tiled_screenshot` function used in `webdriver.py` for dashboard
screenshots and could lead to incomplete or failed screenshot generation in
production. Add a check for width in the condition to ensure both dimensions
are validated.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
if clip_height <= 0 or current_element_box["width"] <= 0:
````
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/36027#issuecomment-3498304539>#2fd6a4</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
