Bug 181279 – Web Inspector: Find next / previous within a resource content view does not have bouncy highlight when editor scrolls

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Find next / previous within a resource content view does not have bouncy highlight when editor scrolls

Steps to Reproduce:
1. Inspect webkit.org
2. Show Debugger tab
3. Select "global.js"
4. Show Find banner
5. Search for "document"
5. Search forward a few times
  => When the ContentView scrolls to review a new result there is no bouncy highlight

Notes:
- We hide the bouncy highlight on scrolls, but the scroll to the new position hid the bouncy highlight!
- We also appear to be leaking scroll handlers if we search quickly

[Radar WebKit Bug Importer]

<rdar://problem/36291097>

[Joseph Pecoraro]

Created attachment 330445 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 330445 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=330445&action=review

> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:1113
> +            if (scrollCount < 2)

This seems very risky.  Is there a way we can add an "ignoreScroll" member variable that is true whenever we are moving to a highlight position?

[Joseph Pecoraro]

Comment on attachment 330445 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=330445&action=review

>> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:1113
>> +            if (scrollCount < 2)
> 
> This seems very risky.  Is there a way we can add an "ignoreScroll" member variable that is true whenever we are moving to a highlight position?

I tried to explain why I didn't do that in the ChangeLog. I think that would be more code, and more error prone, then just triggering this removal after a couple scroll events. I didn't spend too much time thinking about how we could determine if the scroll was programmatic or not. The intent of this scroll handler is if the user is scrolling while the bouncy highlight is up then hide the bouncy. I think that is still achieved with the idea of firing it after 2 scroll.

The best solution would be to re-position the bouncy highlight instead of removing it. I could try to do that.

[Joseph Pecoraro]

Created attachment 330504 [details]
[PATCH] Proposed Fix

[Blaze Burg]

Comment on attachment 330504 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 330504 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 330504

Committed r226517: <https://trac.webkit.org/changeset/226517>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.