1440229 - not enough focusing range of developer tool's search

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[SeulGi Choi]

Attachment: search.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180206200532

Steps to reproduce:

I tried to search element's ID on developer's tool.
attached file explains,
1. when the keyword is behind horizontal scroll.
2. when the keyword is visible on the screen.


Actual results:

when the search keyword is right behind of horizontal scroll, it doesn't move the vertical scroll to let it displays on the screen.


Expected results:

vertical scroll should be moved to make the search result visible on the screen. so user is able to know where the keyword is without scrolling.

Comment 1

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID 	20180226230123

I was able to reproduce this on the latest Firefox release and the latest Nightly using Windows 10 x64, Windows 7 x64, Mac 10.12.6 and Arch Linux x64. I've loaded https://www.mozilla.org/en-US/firefox/60.0a1/whatsnew/?oldversion=58.0.2, opened the Inspector, resized the browser window so that a horizontal scrollbar appeared, searched for "endif" and the third result was displayed under the scrollbar.

The issue is only reproducible on Mac if the scrollbars are set to always show.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

We're using our scrollIntoViewIfNeeded helper https://searchfox.org/mozilla-release/rev/55854d1d5fea58eb257bedfe707f2228556943a3/devtools/client/inspector/markup/markup.js#1700,1716

showNode(node, { centered = true, slotted, smoothScroll = false } = {}) {
...
      scrollIntoViewIfNeeded(container.editor.elt, centered, smoothScroll);

which uses window.scrollBy
We could probably try to use Element#scrollIntoViewIfNeeded instead, which seems to handle the case from comment 0 just fine

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1440229 - [devtools] Scroll to first search result matching Range in Markup view. r=#devtools.

When navigating through search results, we were selecting nodes and scrolling
them into view, centered vertically.
This is not working well in some cases:

• if the node itself is taller than the markup view, we're scrolling in the middle
  of the node and might miss the actual search results
• if the node is wider than the markup view, since we're not scrolling horizontally,
  we might miss a search result at the very right of the node

To fix those issue, we're taking advantage of nsISelectionController.scrollSelectionIntoView,
putting the first search result Range in the selection so we scroll to the right location.
It's okay to put the Range in the selection since the user can't have any text
selected at this point.

A couple test cases are added to cover this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/30daf57a0d52
https://hg.mozilla.org/integration/autoland/rev/c0e8ea9cf124
[devtools] Scroll to first search result matching Range in Markup view. r=devtools-reviewers,jdescottes.

Comment 6

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/c0e8ea9cf124