fix(person): dismiss composer overlay to prevent dialog collision#464
fix(person): dismiss composer overlay to prevent dialog collision#464devag7 wants to merge 2 commits into
Conversation
Greptile SummaryThis PR updates the LinkedIn connect flow to clear message UI before opening the invite dialog. It changes:
Confidence Score: 3/5This should be fixed before merging.
Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
linkedin_mcp_server/scraping/extractor.py:2255-2257
**Fast dismissal can hang**
`fast=True` sets `timeout` to `0`, but Playwright-style `wait_for(..., timeout=0)` disables the timeout instead of doing a quick visibility check. If an `aside` close/dismiss button exists in the DOM but the first match is hidden or minimized, `_locator_is_visible` can wait forever before `connect_with_person` navigates to the invite deeplink. That leaves the connection flow stuck before the invite dialog opens.
Reviews (2): Last reviewed commit: "fix(person): restrict dismissal to messa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens the connection-request flow against LinkedIn’s messaging composer overlay interfering with the invite dialog detection and navigation.
Changes:
- Update
_dialog_is_opento ignore messaging composer overlays that match[role="dialog"]. - Dismiss the messaging composer overlay before navigating to the custom-invite deeplink.
- Add an async test asserting the dismiss happens before navigation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_scraping.py | Adds a regression test that asserts overlay dismissal occurs before invite navigation. |
| linkedin_mcp_server/scraping/extractor.py | Filters composer overlays out of dialog detection and proactively dismisses messaging UI before invite navigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| count = await locator.count() | ||
| if count == 0: | ||
| return False | ||
| await locator.first.wait_for(state="visible", timeout=timeout) | ||
| return True | ||
|
|
||
| # Filter out composer overlays (they have [role="dialog"] but shouldn't count as modal dialogs) | ||
| for i in range(count): | ||
| el = locator.nth(i) | ||
| if await el.is_visible(timeout=timeout): | ||
| # Check if it contains the messaging close button | ||
| if await el.locator(_MESSAGING_CLOSE_SELECTOR).count() > 0: | ||
| continue | ||
| return True | ||
| return False |
| # Issue 432: Clear any message composer overlay which might overlap the invite dialog | ||
| await self._dismiss_message_ui() | ||
| await self._navigate_to_page(invite_url) |
| async def mock_nav(*args, **kwargs): | ||
| calls.append("nav") | ||
|
|
||
| async def mock_dismiss(*args, **kwargs): |
| patch.object( | ||
| extractor, | ||
| "_navigate_to_page", | ||
| new_callable=AsyncMock, | ||
| side_effect=mock_nav, | ||
| ), |
| assert calls[0] == "dismiss" | ||
| assert calls[1] == "nav" |
| timeout = 0 if fast else 750 | ||
| if not await self._locator_is_visible(selector, timeout=timeout): | ||
| return |
There was a problem hiding this comment.
fast=True sets timeout to 0, but Playwright-style wait_for(..., timeout=0) disables the timeout instead of doing a quick visibility check. If an aside close/dismiss button exists in the DOM but the first match is hidden or minimized, _locator_is_visible can wait forever before connect_with_person navigates to the invite deeplink. That leaves the connection flow stuck before the invite dialog opens.
Prompt To Fix With AI
This is a comment left during a code review.
Path: linkedin_mcp_server/scraping/extractor.py
Line: 2255-2257
Comment:
**Fast dismissal can hang**
`fast=True` sets `timeout` to `0`, but Playwright-style `wait_for(..., timeout=0)` disables the timeout instead of doing a quick visibility check. If an `aside` close/dismiss button exists in the DOM but the first match is hidden or minimized, `_locator_is_visible` can wait forever before `connect_with_person` navigates to the invite deeplink. That leaves the connection flow stuck before the invite dialog opens.
How can I resolve this? If you propose a fix, please make it concise.
Fixes #432. Calls _dismiss_message_ui before navigating to the connect deeplink to prevent the composer overlay from intercepting the dialog.