IntersectionObserver doesn't detect icons in canvas elements

macau23 commented 5 years ago

I created bug https://github.com/huashengdun/webssh/issues/90 in a very good web-based ssh client. I am shown a login page, and then I am using my ssh terminal (see screenshot in that bug).

The problem is that the terminal is covered in keepassxc icons.

Should the icons still be visible in this case? If not, what is the expected way of hiding them in other applications?

(It seems to me that they should not be visible since the field is not visible)

varjolintu commented 5 years ago

The icon will be optional in the next version, and we will release it soon.

macau23 commented 5 years ago

The icon is good, but I would like to know if it should be visible when the field is not visible?

varjolintu commented 5 years ago

It shouldn't. I'll check the page and see why it's visible.

EDIT: I have no test environment for testing the actual page. So this will have to wait.

macau23 commented 5 years ago

I will give you a test environment, can you keep a copy of the password here? https://privatebin.net/?c91ac22e25936ff4#9AM48ewJD7WAU13wSzcFu74tLCL8o2yLNUf3KjcEKWhe

Once you have it, write here, and I will give you the ip and username to login.

varjolintu commented 5 years ago

Got the password.

macau23 commented 5 years ago

You can login at (removed) using localhost as the hostname, and your github username as the username.

The system is configured only to allow connections to localhost.

varjolintu commented 5 years ago

It works. Thanks!

varjolintu commented 5 years ago

Nothing I can do here (yet). Seems the IntersectionObserver doesn't detect canvas elements that are intersecting with these icons. I haven't found a quick workaround for it.

Tibladar commented 5 years ago

I seem to have the same problem but not with canvas. Decided to write here since the issue title hasn't changed to be canvas specific.

Put this html code in a file: https://paste.ee/p/Fpkxe Click on the buttons. The fields are hidden, but the icons are still there.

Video: https://streamable.com/8kqga

varjolintu commented 5 years ago

@Tibladar Thanks. I'll check it out. Maybe I've missed some check for the input fields.

varjolintu commented 5 years ago

Found the problem. Icons are currently handled with a single object instead of a list, and this is totally wrong if there are multiple icons visible. The object always points to the last icon, which explains why only the last icon disappears with the example code.

I made a fix that creates icons dynamically to a list, and now icon position, visibility etc. are handled separately. And it was easier to create a separate object classes for the icons. I'll clean the code and push a PR soon, maybe tomorrow.

varjolintu commented 5 years ago

Feel free to test the fix :)

macau23 commented 5 years ago

I will test it - how can I get it?

varjolintu commented 5 years ago

Simple instructions:

Download https://github.com/keepassxreboot/keepassxc-browser/archive/fix/icon_handling_refactor.zip
With Firefox, open about:debugging#/runtime/this-firefox and click Load Temporary Add-on. Browse to the unzipped folder and select manifest.json file.
With Chromium, open chrome://extensions, enable Developer Mode from the top-right and click Load unpacked button. Select the unzipped folder.

Tibladar commented 5 years ago

Seems to work fine.

macau23 commented 5 years ago

Doesn't work for me. I did get a warning (not error) when enabling the temporary add-on in Firefox: Warning details... Reading manifest: Error processing version_name: An unexpected property was found in the WebExtension manifest.

@varjolintu does it work for you with the webssh link above?

varjolintu commented 5 years ago

@macau23 Oh yes. I remove the PR pending label for this one, because it doesn't solve the canvas issue. The manifest error is fine. It's just because there's one line for Chromium based browsers, and Firefox doesn't support that officially.

macau23 commented 5 years ago

I will keep the other issue (https://github.com/huashengdun/webssh/issues/90) open for now then.

macau23 commented 5 years ago

@varjolintu I am seeing this elsewhere now, like on Proxmox's web interface.

varjolintu commented 5 years ago

@macau23 Does it help if you add the URL to Site Preferences and select Disable all features for it? It's the only workaround for now.

macau23 commented 5 years ago

@varjolintu The workaround works (but of course I can't autofill my passwords!). I will copy-paste until there is a solution. Thanks.

macau23 commented 4 years ago

Shall I close?

varjolintu commented 4 years ago

@macau23 If you wish. But I would keep this open and modify the title to include the Canvas problem. I haven't found a way to fix it.

Chealer commented 5 months ago

Could you please specify:

which version of KeePassXC-Browser is affected
which symptom(s) this causes

keepassxreboot / keepassxc-browser

IntersectionObserver doesn't detect icons in canvas elements #608