vimium icon indicating copy to clipboard operation
vimium copied to clipboard

No hints on cookie banner

Open lornajane opened this issue 2 years ago • 11 comments

Describe the bug Cookie banner containing one link and two buttons pops up, but vimium doesn't hint those controls so I can't dismiss the banner and use the site. I assumed this was just people using span tags or some nonsense, but it's a site I control and it has a proper hyperlink and button tags. Is there a way I can make this available to me? I am not sure if the popup is loading after vimium, or something else?

To Reproduce Steps to reproduce the behavior:

  1. Go to URL https://www.onetrust.com/cookie-banner-gallery/ (without cookies set for this site)
  2. Generate hints. The cookie banner in the footer is what I'm looking at.

Include a screenshot if applicable.

Browser and Vimium version Firefox 94.0 Vimium 1.67.1

This definitely could be more of a question than a bug report, either way I'm hoping there's an easy fix! Screenshot 2022-06-01 at 12-21-02 Cookie Banner Gallery OneTrust

lornajane avatar Jun 01 '22 11:06 lornajane

I tried the SurfingKeys extension and it seems to work with it.

image

So it doesn't seem to be a technical but an implementation issue.

Would love to see it in Vimium, too!

JulianDeal avatar Jul 14 '22 09:07 JulianDeal

I've noticed it with a lot of cookie banners and newsletter popups that can't be closed via hints. Might try if I can find out why this happens (and possibly a fix), if time allows 😬

wjzijderveld avatar Sep 30 '22 08:09 wjzijderveld

I had a quick look at the cookie banner gallery linked in https://github.com/philc/vimium/issues/4068#issue-1255732393 and this seems to be a z-index issue. 🤔

Screenshot of the OneTrust cookie banner gallery linked above, with 3 annotations listed below

  1. The OneTrust cookie banner has a z-index of 2147483645.
  2. The Vimium hints have z-indices of 2140000001.
  3. Vimium hints are added to the DOM, but aren't visible.

ravindUwU avatar Sep 21 '23 06:09 ravindUwU

@ravindUwU That's a great finding!

I made a fork and changed the z-index to the maximum possible value of 2147483647.

https://github.com/JulianDeal/vimium/tree/new-z-max

So far it works on the websites I tested and had no issues.

@philc What is the reason for several layers of link hints? One z-index seems to work just fine, even with overlapping.

Edit: Tested some more cookie banners:. It somehow doesn't work with https://devowl.io/wordpress-real-cookie-banner/ although they only have a z-index of 999999.

JulianDeal avatar Sep 21 '23 23:09 JulianDeal

@JulianDeal I can't seem to find a cookie banner on the page you linked, but there is a "try in sandbox" button that redirects to https://try.devowl.io, which shows a cookie banner. This one is shown within a <dialog>, which seems to have its own "layer" in the browser and ~so I don't think Vimium can do anything here 🤔~ I have a few thoughts on how we might be able to implement link hints for dialogs at https://github.com/philc/vimium/issues/3031#issuecomment-1730693295.

ravindUwU avatar Sep 22 '23 01:09 ravindUwU

Regarding the layers: I found the reason for stacking the layers: #757. Hitting Space will rotate the overlapping link hints. So when all link hints have the same z-index, rotating won't work.

To fix this issue I am making changes to the code right now, so that if the max z-index of the link hints parent elements is greater than 240000001, it will increase Vimium's z-index above this max value. So in fringe cases the rotation won't work anymore, but that's better than not seeing the link hints in the first place. Will continue this weekend.

@ravindUwU Maybe I got the cookie banner because I am located in the EU. Or try deleting your cookies. __

JulianDeal avatar Sep 22 '23 18:09 JulianDeal

This PR should fix the issue with obscured elements. The page mentioned by @lornajane shows it perfectly. After accepting the cookies, there still is a button with z-index of 2147483646 in the left bottom corner. The new logic places its link hint on top while the overlapping link hints in the top right corner are still able to rotate.

We could also change the code to only place the link hint a layer above. This would also hide most link hints of non visible links. But I wanted to change the original functionality as little as possible.

JulianDeal avatar Sep 23 '23 14:09 JulianDeal

Sidenote: reading about how stacking occurs without z-index, I guess Vimium could rotate markers by repositioning elements within the container. Something like https://stackblitz.com/edit/ravinduwu-misc-rotate-vimium-marker-no-z?file=script.js%3AL6

ravindUwU avatar Sep 24 '23 05:09 ravindUwU

@ravindUwU: This is a good solution, too. I will mention it in as an alternative implementation in the PR.

JulianDeal avatar Sep 25 '23 07:09 JulianDeal

Guys, this is great investigative work. Thanks @JulianDeal for the PR.

Looking at the PR and @ravindUwU's proposed change, it seems to me simpler overall if we could manage rotation by modifying the child's index within its parent. Then we wouldn't need to manage z-indices, or document how fringe cases are expected to work.

So in summary:

  1. Set the z-index of all link hints to the max value.
  2. Implement rotation by moving the last hint in a stack to be first in its container, as @ravindUwU has done in his POC.

As a side note, we should confirm that this modification doesn't unintentionally modify the current stacking UX (i.e. if today link A is shown on top of link B in a stack on some example page, this PR shouldn't change that).

@JulianDeal can you modify your PR to take this approach?

philc avatar Sep 26 '23 04:09 philc

@philc: I will. I worked on issue #3031 in the meantime. But I will do it presumably until the weekend.

JulianDeal avatar Sep 27 '23 10:09 JulianDeal