259640 - Find Toolbar's highlight mode should show matches next to or on top of scrollbar

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, enhancement, P3)

Description

[Keith Lea]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10

Some text editors will show matches of a search string next to the scrollbar, so
you can see quickly how many matches there are, and where they are located on
the page. I'd like Firefox's find toolbar to do this as well.

I've attached a screenshot of how IntelliJ IDEA does this.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Keith Lea]

Attachment: Screenshot from IntelliJ IDEA showing matches along the scrollbar

Comment 2

[Keith Lea]

Comment on attachment 159011 [details]
Screenshot from IntelliJ IDEA showing matches along the scrollbar

The green lines next to the scrollbar show where the matches are on the page.

Comment 3

[Jesse Ruderman]

*** Bug 303964 has been marked as a duplicate of this bug. ***

Comment 4

[Ng Ming Hong]

But when a page is not long enough, there are no scrollbar...

Comment 5

[Jesse Ruderman]

On non-scrolling pages, you don't need this feature, because you can spot the yellow areas without scrolling.

Comment 6

[Peter Kasting]

I'm going to take this bug so maybe someday I can do it.  Way too hard for now, but it would be very nice to have.

Marking depends on 77790 as the right solution here may involve being able to style the scrollbar.

Comment 7

[Peter Kasting]

Not going to be getting to this one.

Comment 8

[Peter Kasting]

Oops, meant to unassign :P

Comment 13

[Siddhartha Dugar [:sdrocking]]

https://addons.mozilla.org/en-US/firefox/addon/scrollbar-search-highlighter/
does this in some way.

Comment 15

[Colby Russell :crussell (no longer Mozilla-ing)]

Bug 384458, comment 4 is relevant.

Comment 16

[Luís Miguel [:quicksaver] (not reading bugmail)]

For anyone concerned, https://addons.mozilla.org/en-US/firefox/addon/findbar-tweak/ does this by placing an overlay over the scrollbar area with the found matches, in a similar way as the current WIP patch at bug 384458 (element inside the browser stack).

To whomever eventually takes this, don't forget frames! Pages with large iframes will likely be just wrong, and frameset based pages won't actually have a "main" scrollbar to use for this. The latest 1.4-beta of FBT tries to address this by placing similar boxes into the content that overlay the frame elements where their scrollbars should be.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Luís Miguel [:quicksaver] from comment #16)
> For anyone concerned,
> https://addons.mozilla.org/en-US/firefox/addon/findbar-tweak/ does this by
> placing an overlay over the scrollbar area with the found matches, in a
> similar way as the current WIP patch at bug 384458 (element inside the
> browser stack).
> 
> To whomever eventually takes this, don't forget frames! Pages with large
> iframes will likely be just wrong, and frameset based pages won't actually
> have a "main" scrollbar to use for this. The latest 1.4-beta of FBT tries to
> address this by placing similar boxes into the content that overlay the
> frame elements where their scrollbars should be.

Luis, would you consider writing a patch for this? I looked at Chrome Canary and they don't show the matches on their scrollbar if they are inside of an iframe. We could implement the simple approach by only showing matches in the top-level frame on the top-level frame's scrollbar and we would be no worse than what is shipping today in Chrome Canary.

You can use the new FinderHighlighter.jsm module which should make this more straightforward to implement. Mike de Boer can help you with navigating FinderHighlighter.

Comment 20

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #19)
> Luis, would you consider writing a patch for this?

I would love to! But realistically speaking I won't be able to even dream about hacking on Firefox code like this any time soon. Likely not before 2017 even... :(

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 259640 - Hacky proof of concept to have Find Toolbar's highlight mode should show matches on top of scrollbar.

Review commit: https://reviewboard.mozilla.org/r/80472/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80472/

Comment 22

[Mike de Boer [:mikedeboer]]

Comment on attachment 8793871 [details]
Bug 259640 - Hacky proof of concept to have Find Toolbar's highlight mode should show matches on top of scrollbar.

https://reviewboard.mozilla.org/r/80472/#review79350

::: toolkit/content/minimal-xul.css:104
(Diff revision 1)
> -scrollbar, scrollbarbutton, scrollcorner, slider, thumb, scale {
> +marker {
> +  -moz-binding: url(chrome://global/content/bindings/scrollbar.xml#marker);
> +  display: -moz-box !important;
> +}
> +
> +scrollbar, scrollbarbutton, scrollcorner, slider, thumb, marker, scale {

I think the original idea was to have this in alphabetical order... or something :)

::: toolkit/content/widgets/scrollbar.xml:14
(Diff revision 1)
> -  
> -  <binding id="thumb" extends="xul:button" />
>  
> +  <binding id="thumb" extends="xul:button"/>
> +  <binding id="marker" extends="xul:button"/>
> +  <bidning id="markers" extends="xul:box"/>

Did `bidning` ever work?

::: toolkit/modules/FinderHighlighter.jsm:1048
(Diff revision 1)
>      dict.modalHighlightAllMask.setCutoutRectsForElement(kMaskId, allRects);
> +    this._setScrollbarMarkers(window, allRects, width, height);
> +  },
> +
> +  _setScrollbarMarkers(window, rects, width, height) {
> +    if (!window || !window.document || !window.document.documentElement) {

Instead of a separate method for this, what do you think about the following ideas?

 * Create a helper method that fetches all the scrollbar nodes, like the first bootstrap part here, something like `_getScrollbarMarkerNodes()`
 * Integrate with the for-loop in `_repaintHighlightAllMask()` to lose the overhead of another loop
 * Lazily create marker nodes and re-use the ones that are already there. After the loop, hide the marker nodes that you didn't need, if any.
 * Use `marker.onclick = e => {}` to make sure there's only one event listener that always does what you'd expect
 * For the scrolling behavior, use `range.startContainer.scrollIntoView()` to have it work (I think almost) like regular find-in-page navigation
 * Percentage-based positioning would be ultra-nice... but the highlights repaint after a window resize anyways, so you get repositioning for 'free'.
 * `parseFloat` doesn't know about a second 'base' argument, but I think you don't need that cast anyway.

Comment 23

[Mike de Boer [:mikedeboer]]

Super cool that you're trying to make this work! I'm rolling my thumbs with impatience... exciting!

Comment 24

[avada]

What(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #21)
> Created attachment 8793871 [details]
> Bug 259640 - Hacky proof of concept to have Find Toolbar's highlight mode
> should show matches on top of scrollbar.
> 
> Review commit: https://reviewboard.mozilla.org/r/80472/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/80472/

What's this like visually?

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to avada from comment #24)
> What(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #21)
> > Created attachment 8793871 [details]
> > Bug 259640 - Hacky proof of concept to have Find Toolbar's highlight mode
> > should show matches on top of scrollbar.
> > 
> > Review commit: https://reviewboard.mozilla.org/r/80472/diff/#index_header
> > See other reviews: https://reviewboard.mozilla.org/r/80472/
> 
> What's this like visually?

This is similar to FindBar Tweak, https://addons.cdn.mozilla.net/user-media/previews/full/165/165756.png?modified=1455331568

Comment 26

[avada]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #25)
> This is similar to FindBar Tweak,
> https://addons.cdn.mozilla.net/user-media/previews/full/165/165756.
> png?modified=1455331568

I'm familiar with that. I have an idea for an enhancement. The scrollbar highlits could have some transparency. That way it would show if multiple results are within a single highlight. (Which can easily happen on a long page)

Comment 29

[avada]

Any chance of this happening, before the addon that brings this functionality stops functioning in v57?

Comment 30

[Lucas Costi]

+1 This is a pretty core feature for me. It would be a big disappointment if 57 didn't have this functionality either in the core or something that could be enabled via an addon.

Comment 31

[Martin Strobl]

The "TextMarker Go" add-on (https://addons.mozilla.org/cs/firefox/addon/textmarker-go/?src=api) for Firefox Quantum has the scrollbar flag functionality, so maybe it's developer could share some knowledge on how to make the scrollbar highlight available also for the Find dialog.

Comment 33

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 34

[BugBot [:suhaib / :marco/ :calixte]]

Moving to p3 because no activity for at least 24 weeks.
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 36

[p.bodnar]

Apart from the "TextMarker Go" add-on (https://addons.mozilla.org/cs/firefox/addon/textmarker-go/), there is also another add-on with the same purpose: "HighlightAll" (https://addons.mozilla.org/en-US/firefox/addon/highlightall/). One just needs to activate the "Custom highlighting" mode in the add-on's preferences. So maybe the need to make this a native feature diminishes, although it is possibly the only feature which is really missed by users which come to Firefox from Chrome - my subjective point of view though...

Comment 38

[cardinleejh]

(In reply to p.bodnar from comment #36)

Apart from the "TextMarker Go" add-on (https://addons.mozilla.org/cs/firefox/addon/textmarker-go/), there is also another add-on with the same purpose: "HighlightAll" (https://addons.mozilla.org/en-US/firefox/addon/highlightall/). One just needs to activate the "Custom highlighting" mode in the add-on's preferences. So maybe the need to make this a native feature diminishes, although it is possibly the only feature which is really missed by users which come to Firefox from Chrome - my subjective point of view though...

TextMarker Go is not a "highlight search results in scrollbar" extension. It is a "display bookmarked text in scrollbar" extension.

HighlightAll does not integrate with the search functionality, and hence does not support paging to each result.

Comment 39

[yuweizhu90]

(In reply to cardinleejh from comment #38)

(In reply to p.bodnar from comment #36)

Apart from the "TextMarker Go" add-on (https://addons.mozilla.org/cs/firefox/addon/textmarker-go/), there is also another add-on with the same purpose: "HighlightAll" (https://addons.mozilla.org/en-US/firefox/addon/highlightall/). One just needs to activate the "Custom highlighting" mode in the add-on's preferences. So maybe the need to make this a native feature diminishes, although it is possibly the only feature which is really missed by users which come to Firefox from Chrome - my subjective point of view though...

TextMarker Go is not a "highlight search results in scrollbar" extension. It is a "display bookmarked text in scrollbar" extension.

HighlightAll does not integrate with the search functionality, and hence does not support paging to each result.

It really seems to me that the Firefox team does not care about this. This issue is opened from 15 years ago and they are always busy with something else in these 15 years. This is the feature in "find in page" functionality that falls behind Chrome for years. They are trying to work on Pocket, Gecko, Send, Monitor, etc. features which only a few percent users use daily. However, they ignore this long lasting issue in "find" feature people use everyday while pretending this issue is resolved by plugin.

There must be something seriously wrong in the Mozilla Enterprise culture and their management team. I guess their phylosophy might be: be fond of the new and tired of the old.

I don't believe the Firefox team will ever implement this feature in the page search.

Comment 40

[federico.piazza]

I think Chrome does this since the beginning... is there any update for this? I hate using Chrome when I need this feature.

Comment 42

[Chi]

Can any Mozilla developer suggest how do you jump to the correct position quickly, without this feature, when searching on API reference pages which usually contains dozens, if not hundreds, of matches and screens?

My experience: on API reference pages, it is quite common that a method name is referred dozens of times in many other methods description, but in 98% cases, the highest density highlighted position in scrollbar (of course, on Chrome) is the correct position for the searched method name.

Missing this feature force me to open Chrome for just Find (on page) feature...

Comment 43

[lanzwjohn]

I use this feature daily in my workflow.... I am currently evaluating Firefox to determine if I can switch from Chrome (I like FF's privacy standards) however this is surprisingly a big issue for me. I would entertain helping to add this.

Comment 44

[Vipul Agarwal]

This is super useful feature. Please prioritize this work if possible.

Comment 45

[avada]

Is there functioning workaround (addon/script/etc) for this?
The HighlightAll addon sort of is, but it's a fair bit less comfortable to use like that.

Comment 46

[Poopooracoocoo]

this issue is almost as old as me...

Comment 47

[tallyo]

how can this feature not be added and its requested for over a decade long.

just what is firefox dev thinking, do they not want market share from chrome? and to think they boast themselves to be the browser for power users.

Comment 48

[Poopooracoocoo]

you used to be able to add anything other browsers had to firefox, but that was 5 years ago.

Comment 49

[avada]

(In reply to tallyo from comment #47)

how can this feature not be added and its requested for over a decade long.

just what is firefox dev thinking, do they not want market share from chrome? and to think they boast themselves to be the browser for power users.

Are yo new to Firefox or Bugzilla? Everything that's not on the trendy train takes several years to decades to infinity to implement with the Mozilla MO. Even if they're low scale changes.

Comment 50

[Neil Deakin]

Attachment: Bug 259640, add chrome only method to add scrollbar tick marks to a window, r=smaug,tnikkel

Comment 51

[Neil Deakin]

Attachment: Bug 259640, paint tick marks on vertical sliders for the root scroll frame of the document if any have been assigned to the window, r=tnikkel

Depends on D100938

Comment 52

[Neil Deakin]

Attachment: Bug 259640, update the scroll marks when a find in page highlight in completed, using the positions of the highlights as the marker points, r=mikedeboer

Depends on D100939

Comment 53

[Neil Deakin]

Thought it might be fun to implement this during the break. With the above patches, the tick marks are drawn in a window's scrollbar using the highlight colour which defaults to something magenta-like. Note that the 'Highlight All' toggle on the findbar needs to be on for the tick marks to appear.

Comment 54

[Neil Deakin]

Attachment: Bug 259640, add some reftests that verify that scrollbar marks are drawn, r=tnikkel

Depends on D100940

Comment 55

[Neil Deakin]

Attachment: Bug 259640, adjust the height of the find marks on scrollbars based on dpi, r=tnikkel

Depends on D100940

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67faf54a31a1
add chrome only method to add scrollbar tick marks to a window, r=smaug,tnikkel
https://hg.mozilla.org/integration/autoland/rev/2e218cf0cf63
paint tick marks on vertical sliders for the root scroll frame of the document if any have been assigned to the window, r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/fd4c496cee86
update the scroll marks when a find in page highlight in completed, using the positions of the highlights as the marker points, r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/3bece74bc551
adjust the height of the find marks on scrollbars based on dpi, r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/9e6cd269d773
add some reftests that verify that scrollbar marks are drawn, r=tnikkel

Comment 57

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/67faf54a31a1
https://hg.mozilla.org/mozilla-central/rev/2e218cf0cf63
https://hg.mozilla.org/mozilla-central/rev/fd4c496cee86
https://hg.mozilla.org/mozilla-central/rev/3bece74bc551
https://hg.mozilla.org/mozilla-central/rev/9e6cd269d773

Comment 58

[Mike de Boer [:mikedeboer]]

HUGE applause to Neil for spending his spare time on this elegant implementation of scroll marks!! I know that at least :jaws and I have been looking at this bug with starry eyes for years and - considering the age of this one - there must be many, many others.

Thanks so much, Neil, and keep the awesome contributions coming! ;-)

Comment 59

[tustamido]

How is this supposed to work? It's not working for me (Windows 10), there's no marks in scrollbar after using find with some results. I didn't notice any difference. findbar.highlightAll = true.
I asked to other Nightly users and the two answers so far was that it isn't working for them too.

Comment 60

[Neil Deakin]

Attachment: Bug 259640, fix highlightAll reference, r=mikedeboer

Comment 61

[Neil Deakin]

Bah, just put up a fix for the typo. Hopefully, this should fix things.

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d8a1ee8100c
fix highlightAll reference, r=mikedeboer

Comment 63

[tustamido]

(In reply to Neil Deakin from comment #61)

Bah, just put up a fix for the typo. Hopefully, this should fix things.

Fixed. Tested 8d8a1ee8100c autoland build and now it's working, thank you.

Comment 64

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8d8a1ee8100c

Comment 65

[Sören Hentzschel]

First: Thanks for working on it! The bad news: at least on macOS the implementation is very fragile:

Very often it doesn't highlight anything. When it works and I change the search term the highlights do not always update. Example: I searched for a word with many occurences, they get highlighted correctly. Then I changed the search term to another one with only two occurences. The number of highlights changed to two (as expected). Then I changed the search term back to the first one again and instead of many highlights Firefox kept the two highlight of the previous search term.

Sometimes resizing the browser window fixed the highlights (seems to be the opposite behaviour of bug 1690791), sometimes resizing changes nothing.

It's also unexpected that there also highlights even with a disabled "highlight all" feature.

I posted it here because I am not sure if these are all related problems or if these are different bugs.

Comment 66

[Neil Deakin]

Bug 1690783 will likely fix the issues with the highlight not being updated correctly after changing the search.

Comment 67

[Sören Hentzschel]

Thanks! I will follow bug 1690783 and file new bugs for remaining problems (if any!) once bug 1690783 is closed. :)

Comment 68

[Daniel Bodea [:danibodea]]

I can confirm that the bar with tick highlights appears when searching existing strings in a window that has a vertical scroll bar, but I can also reproduce the logged issues that block this bug.

Will we use this bug as a meta bug for the remaining issues/tasks? I am thinking that we could block bug 565552 or, more likely, bug 1271782 with the remaining issues and close this current one as verified. What do you think?

Comment 69

[u662737]

Hi this feature doesn't work on pdf files.

Comment 70

[Julien Cristau [:jcristau]]

This is in the 87.0beta relnotes as:

“Find in page” can now display marks next to the scrollbar corresponding to the positions of matches.

Comment 71

[Paul]

I am currently on 88.0a1 (2021-03-10) (64-bit) and it doesn't work at all on https://elixir.bootlin.com/linux/v5.8.11/source/kernel/sched/fair.c.

Comment 72

[Emilio Cobos Álvarez (:emilio)]

(In reply to Paul from comment #71)

I am currently on 88.0a1 (2021-03-10) (64-bit) and it doesn't work at all on https://elixir.bootlin.com/linux/v5.8.11/source/kernel/sched/fair.c.

That doesn't work in other browsers either, because that page is hiding the root scrollbars, then having a full wrapper over the whole content's of the page... A bit unfortunate IMO. It could be made to work, perhaps, but for Gecko that scrollbar is just the scrollbar of another <div>.

Comment 73

[avada]

Visibility is rather poor on dark themed websites with a dark scrollbar. such as right now on bugzilla.

I'm guessing it will be improved some time in the future. Until then, can I manually change the color or something?

Comment 74

[Shane Hughes [:aminomancer]]

the color is determined by preference: ui.textHighlightBackground
see here: https://searchfox.org/mozilla-central/source/layout/xul/nsSliderFrame.cpp#294
unfortunately that will affect highlights everywhere, so you can't independently change this particular color.

would you guys consider making the slider marks toggled by a preference? the way it's implemented pushes just about any kind of customization way out of reach.

Comment 75

[Shane Hughes [:aminomancer]]

to clarify, some kind of preference to change the color, opacity, and/or thickness would be equally useful. I too find the current values very difficult to see, regardless of color. it's just the combination of 0.3 alpha and very thin bars that's making it blend in.

Comment 76

[mimecry]

i concur with the above sentiments, the feature as it is right now isn't the most helpful.

Comment 77

[mimecry]

seems like i am unable to attach images, so here's a direct link to a screenshot showing the implementation in Chromium which i think is a lot more visible

https://i.imgur.com/oIr7Hig.png

Comment 78

[Neil Deakin]

The marks appearance is covered by bug 1690129.

Comment 79

[mimecry]

thank you very much, i'll keep an eye on that link