Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Form autofill popup should appear under the focused field #3359

Closed
bbondy opened this issue Aug 24, 2016 · 13 comments · Fixed by brave/muon#44
Closed

Form autofill popup should appear under the focused field #3359

bbondy opened this issue Aug 24, 2016 · 13 comments · Fixed by brave/muon#44

Comments

@bbondy
Copy link
Member

bbondy commented Aug 24, 2016

Instead it is currently showing up in the middle for me. For @darkdh it appears at the correct location though. Maybe a DPI issue?

screenshot 2016-08-24 02 22 36

@bbondy bbondy added this to the 0.11.6dev milestone Aug 24, 2016
@luixxiul
Copy link
Contributor

Same here on Windows 7 64 bit

@darkdh
Copy link
Member

darkdh commented Aug 24, 2016

That's weird. My location is correct
screen shot 2016-08-24 at 16 28 19

@bsclifton
Copy link
Member

bsclifton commented Aug 24, 2016

I tested this too (after running rm -rf ./node_modules/ && rm -rf ~/.electron/ && npm install) and it looks good to me (macOS):
screen shot 2016-08-24 at 9 00 30 am

Let me try on Windows

@bsclifton
Copy link
Member

doh- it happened for me on Windows 😦

image

@bsclifton
Copy link
Member

@darkdh mentioned position is emitted from here:
https://github.com/brave/electron/blob/master/atom/browser/autofill/atom_autofill_client.cc#L166

I can help look at this issue

@bsclifton bsclifton self-assigned this Aug 24, 2016
@bsclifton
Copy link
Member

@bbondy did you have any specific STR? I can't repro on Mac

@bsclifton
Copy link
Member

bsclifton commented Aug 24, 2016

I'm reliably able to repro now- thanks to @bridiver for suggestion when I was talking it out

Offset appears to be adding the offset of the window in addition to the frame's x/y. Looking at a fix now...

@bsclifton
Copy link
Member

bsclifton commented Aug 24, 2016

PRs submitted for brave/electron and brave/browser-laptop

Root cause:

  • position being sent back was the top left of the textbox
  • need to add the height of the textbox, so it goes below
  • @darkdh had the right idea by getting the client area;
    • however, we didn't want the X/Y... we needed the width/height
    • this can be compared against the frame's width/height
    • offset is then correct

@luixxiul
Copy link
Contributor

On 0.11.6 Beta5 (Windows 7 64 bit JP)
clipboard01
Hasn't this been fixed?

@luixxiul
Copy link
Contributor

Also, though I click the suggestion, it is not filled in the field.

@darkdh
Copy link
Member

darkdh commented Aug 27, 2016

This is brave built-in password manager, I assume.

@luixxiul
Copy link
Contributor

Yes it is. I just thought they were similar.

@bridiver
Copy link
Collaborator

autofill seems to track usernames as well, but the location is correct on mac

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants