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

Link hints: false positives and scrollable divs #2069

Merged
merged 4 commits into from
Mar 28, 2016

Conversation

smblott-github
Copy link
Collaborator

This builds on #2048 (global link hints) adding two new features:

Filter out false positives

We mark any element with a class name containing the text button as clickable. Many of these are wrappers around real clickables. E.g....

<span class="buttonWrapper"><input type="button"/></span>

Here, we treat the outer span as a false positive (because a close descendent is a real clickable). This has two effects:

  1. We get fewer useless hints.
  2. Because we have fewer hints, it is less common that hint markers overlap, so the hint markers for the real clickables are less likely to be obscured by useless hint markers.
Make scrollable divs selectable

Here's an example (using filtered hints):

snapshot

Fixes #425.
Fixes #2056.

(I intend to merge this directly; this PR is just for visibility.)

Fixes philc#425.

Conflicts:
	content_scripts/scroller.coffee
We recognise elements with a class names containing the text "button" as
clickable.  However, often they're not, they're just wrapping a
clickable thing, like a real button.

Here, we filter out such false positives.

This has two effects:

- It eliminates quite a number of real false pasitives in practice.
- With fewer hints close together, fewer hint markers are obscured by
  the hints from (non-clickable) wrappers.  This reduces the need for
  rotating the hint stacking order, e.g philc#1252.
We should start by checking the *parent* of the candidate descendant.
Testing the `scrollHeight` is cheaper than testing scrollability.  There
are a lot of non-scrollabile divs, so it makes sense to use this cheaper
test as a filter.
@smblott-github smblott-github merged commit 9700c9f into philc:master Mar 28, 2016
@smblott-github smblott-github deleted the global-link-hints-++ branch March 28, 2016 05:30
@vnikk
Copy link

vnikk commented Oct 25, 2017

hi, could you please comment on how to use it. I cant find any documentation on this and it's not clear from the code. thanks

@smblott-github
Copy link
Collaborator Author

There's nothing that you (the user does). The text Scroll appears if you're using filtered hints (under advanced options on the options page, Use the link's name and characters for link-hint filtering).

@vnikk
Copy link

vnikk commented Oct 27, 2017

thank you. is it possible to apply without hint filtering?

@smblott-github
Copy link
Collaborator Author

The hint is still there with alphabet hints, you just don't have the text "Scroll" next to it.

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

Successfully merging this pull request may close these issues.

Link-hint false positives Make it possible to focus a scrollable div without using the mouse
2 participants