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

Labels are not clickable anymore #3492

Open
stianjensen opened this issue Feb 11, 2020 · 14 comments
Open

Labels are not clickable anymore #3492

stianjensen opened this issue Feb 11, 2020 · 14 comments

Comments

@stianjensen
Copy link

Describe the bug
When pressing 'f' to highlight click targets, labels used to be highlighted so that you were able to select for instance custom bootstrap checkboxes (https://getbootstrap.com/docs/4.4/components/forms/#checkboxes)
Since the last version, those checkboxes are no longer selectable with vimium.

To Reproduce
Steps to reproduce the behavior:

  1. Go to URL: https://getbootstrap.com/docs/4.4/components/forms/#checkboxes
  2. Click 'f' on the keyboard
  3. Observe that the checkbox does not get highlighted

Before and after:

Browser and Vimium version
I've seen the bug in both Chrome and Firefox, although I don't have a specific chrome version handy at the moment.

Firefox Version: 73.0b12
Build ID 20200130174018
Vimium 1.65.0, 1.65.1, 1.65.2

(A working version of Vimium is 1.64.6)

@mihmig
Copy link

mihmig commented Feb 17, 2020

span, label and other elements, that have CSS-attribute cursor, might also be clickable:
изображение

@lydell
Copy link

lydell commented Feb 18, 2020

As far as I can tell, those checkboxes are actually standard <input type="checkbox"> so the checkboxes themselves should be detectable by Vimium. @poacher2k You might be interested in this.

@poacher2k
Copy link
Contributor

poacher2k commented Feb 18, 2020 via email

@poacher2k
Copy link
Contributor

poacher2k commented Feb 19, 2020

As far as I can tell, what's going on here is that the input have position: absolute and z-index: -1. No idea why the label doesn't have a link hint, but it don't have that in the before picture either 🤔

@stianjensen
Copy link
Author

Okay, so the change isn't that labels aren't clickable anymore, but rather that hidden checkboxes aren't clickable anymore.

I guess that ignoring hidden checkboxes (and other elements) has other advantages, so you don't simply want to revert that change (I see other webpages with hidden popup menus have improved in the newest version).

But maybe labels in general should get link hints - at least those with the for attribute set?

@poacher2k
Copy link
Contributor

But maybe labels in general should get link hints - at least those with the for attribute set?

Totally agree. Thoughts @philc ?

@lydell
Copy link

lydell commented Feb 19, 2020

Sorry for the confusion – you’re right. The checkbox is hidden in this case, and replaced with label::before and label::after styled as a checkbox.

I was confused becuase my own extension – Link Hintsdoes identify the checkbox as visible and places a hint near it (rather than falling back to adding a hint for the label instead), so I assumed that Vimium had some little bug in its .elementFromPoint usage. But it turned out to be much trickier than that: lydell/LinkHints@704f1d4

I suspect Vimium has never supported adding hints for <label> elements. Vimium identifies <input type="checkbox"> as clickable, but .elementFromPoint() returns the <label> element here, because its pseudo elements are located on top of the checkbox, making Vimium think the checkbox is covered by other elements and therefore shouldn’t get a hint. The reason this worked before is simply because the .elementFromPoint() check didn’t use to exist. You should be able to fix this by using the same workaround as in Link Hints: https://github.com/lydell/LinkHints/blob/704f1d414eae28cce2604584bcf03f65fbb197fe/src/worker/ElementManager.js#L2066-L2068

That being said, it would be good to support hints for <label> elements as well. The only tricky part is to not give hints to <label>s if their form control is visible (otherwise you end up with basically duplicate hints for every form control which is a bit noisy and makes the hints longer). (Yes, Link Hints does this as well ^.^)

@danny-does-stuff
Copy link

danny-does-stuff commented Oct 5, 2022

Any update on this issue? I would love to be able to review a whole Merge Request on Gitlab with vimium but I can't collapse the diffs using the checkbox as I would hope to be able to
image
You can see that there are no link tags on the checkboxes for "Viewed"

@poacher2k
Copy link
Contributor

poacher2k commented Oct 5, 2022

That being said, it would be good to support hints for <label> elements as well. The only tricky part is to not give hints to <label>s if their form control is visible (otherwise you end up with basically duplicate hints for every form control which is a bit noisy and makes the hints longer). (Yes, Link Hints does this as well ^.^)

One option could be to only give it for the label and not the input if the label is a parent of the input, or is referred to with for="inputId". Not really sure what the best solution is though.

Edit: Or check both, and default to the label (if parent / for="") if the input itself isn't visible, or only the input if it is visible. This might be the best of both worlds

@danny-does-stuff
Copy link

For what it's worth, I would much prefer duplicate tags over no tags at all. As you can see in my screenshot there are already plenty of duplicates which is fine

@gdh1995
Copy link
Contributor

gdh1995 commented Oct 10, 2022

@poacher2k Vimium has used a logic to show hint for a <label> only if its bound <input> is not "visible".

@dannyharding10 today I tested Vimium 1.67.1 on https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/6383/diffs and it did show hints for "viewed" checkboxes, so I wonder what's the version of you tested GitLab website. Maybe it's too old to provide standard information about whether checkboxes are clickable or not.

@poacher2k
Copy link
Contributor

poacher2k commented Oct 10, 2022

@poacher2k Vimium has used a logic to show hint for a <label> only if its bound <input> is not "visible".

@dannyharding10 today I tested Vimium 1.67.1 on https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/6383/diffs and it did show hints for "viewed" checkboxes, so I wonder what's the version of you tested GitLab website. Maybe it's too old to provide standard information about whether checkboxes are clickable or not.

Sounds perfect! But I can confirm what @dannyharding10 experiences - the label isn't clickable on this page, at least not in Firefox:
image

The visible checkbox is a :before element, while the actual checkbox has position: absolute; z-index: -1;, meaning the actual checkbox is not visible. If I manually set display: none on the checkbox and press F, the label correctly gets a link hint:

image

@gdh1995
Copy link
Contributor

gdh1995 commented Oct 10, 2022

Yes your analysis is correct, but it's strange that my Firefox 105 on Win11 does give different result:
image

You may try my customized version of Vimium, named Vimium C (https://github.com/gdh1995/vimium-c), and it has many improved details to try its best to find clickable elements.

@trymeouteh
Copy link

Would like to see label elements be supported since label elements can also act as button sometimes

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

No branches or pull requests

7 participants