Re: [PR] fix: Ensure that Playwright tile height is always positive [superset]

Thu, 06 Nov 2025 08:52:11 -0800 


korbit-ai[bot] commented on code in PR #36027:
URL: https://github.com/apache/superset/pull/36027#discussion_r2499872145


##########
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:
+                logger.warning(
+                    "Skipping tile %s/%s due to invalid clip dimensions: "
+                    "width=%s, height=%s (element may be scrolled out of 
viewport)",
+                    i + 1,
+                    num_tiles,
+                    current_element_box["width"],
+                    clip_height,
+                )
+                continue

Review Comment:
   ### Incomplete validation of clip dimensions <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The code only checks for clip_height <= 0 but doesn't validate that 
current_element_box["width"] > 0, yet it skips the tile claiming "invalid clip 
dimensions" for both width and height.
   
   
   ###### Why this matters
   If the element width becomes 0 or negative (which can happen when elements 
are completely out of viewport), the screenshot will still be attempted with 
invalid width dimensions, potentially causing crashes or unexpected behavior in 
the screenshot API.
   
   ###### Suggested change ∙ *Feature Preview*
   Add validation for both width and height dimensions:
   
   ```python
   if clip_height <= 0 or current_element_box["width"] <= 0:
       logger.warning(
           "Skipping tile %s/%s due to invalid clip dimensions: "
           "width=%s, height=%s (element may be scrolled out of viewport)",
           i + 1,
           num_tiles,
           current_element_box["width"],
           clip_height,
       )
       continue
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bdb4dd6f-972e-4eea-b5d2-1c9416565421/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bdb4dd6f-972e-4eea-b5d2-1c9416565421?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bdb4dd6f-972e-4eea-b5d2-1c9416565421?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bdb4dd6f-972e-4eea-b5d2-1c9416565421?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bdb4dd6f-972e-4eea-b5d2-1c9416565421)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:39b1993f-45db-4287-83de-8294e26454ce -->
   
   
   [](39b1993f-45db-4287-83de-8294e26454ce)



-- 
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]

Reply via email to