Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure autofill popup view is > 1x1 in size #36121

Merged
merged 2 commits into from
Jan 27, 2023

Conversation

nornagon
Copy link
Member

Description of Change

Fixes #26667

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes

Release Notes

Notes: none

@nornagon nornagon added no-backport semver/patch backwards-compatible bug fixes labels Oct 24, 2022
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 24, 2022
codebytere
codebytere previously approved these changes Oct 25, 2022
@codebytere codebytere dismissed their stale review October 25, 2022 06:15

Oops meant to comment - it doesn't look like popup_->popup_bounds_.Union returns a gfx::Rect

@@ -224,7 +224,8 @@ void AutofillPopupView::DoUpdateBoundsAndRedrawPopup() {
if (!popup_)
return;

GetWidget()->SetBounds(popup_->popup_bounds_);
GetWidget()->SetBounds(popup_->popup_bounds_.Union(
gfx::Rect(popup_->popup_bounds_.origin(), gfx::Size(1, 1))));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If popup_->popup_bounds_ could be empty, then so would be popup_->popup_bounds_in_view(), which is used in some other places.

@zcbenz
Copy link
Member

zcbenz commented Oct 25, 2022

It was also mentioned in #26667 that passing zero size to BrowserWindow could cause the same problem.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 25, 2022
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
@codebytere codebytere merged commit 85f41d5 into main Jan 27, 2023
@codebytere codebytere deleted the clamp-autofill-popup-size branch January 27, 2023 09:50
@release-clerk
Copy link

release-clerk bot commented Jan 27, 2023

No Release Notes

khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* fix: ensure autofill popup view is > 1x1 in size

ref electron#26667

* Update shell/browser/ui/views/autofill_popup_view.cc

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Feb 28, 2023
* fix: ensure autofill popup view is > 1x1 in size

ref electron#26667

* Update shell/browser/ui/views/autofill_popup_view.cc

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DCHECK in autofill relating to zero-sized windows on macOS
3 participants