Step to reproduce: 1. Open the inspector on a page with multiple links, script, etc. 2. In the search box, enter [src^=http] 3. a drop down appears with the list of matches Expected: Being able to navigate all the node matching my query and having the query as-is in the search box. Actual: Matches an element then navigate through this element on return if it matches or not the pattern. For example, if it matches script in the dropdown, it will continue to navigate through other script elements even if they do not have the `src="http…` attribute value.

This is really bad. The suggestion popup is also all messed up. For instance: - open mozilla.org - open inspector - enter [src^=http] in the search box - the suggestion popup appears and contains one element: [src^=h which is wrong - selecting this element from the suggestion list replaces the input field value with script - even if you don't select the element from the list, pressing <enter> will have the same effect. The search box doesn't support attribute selectors well at all. Confirmed on both nightly (47) and aurora (46).

Filter on CLIMBING SHOES

Should we show the suggestion popup when searching on attribute ? And if so, how ?

The suggestion popup is especially useful for tags, ids and classes (and in fact, I believe it only works with those). So we should not have a popup for things like [src^=http].

Add an ATTRIBUTE search state to better handle attribute's search. Edit test to make sure we handle it right Review commit: https://reviewboard.mozilla.org/r/49509/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49509/

Comment on attachment 8746640 [details] MozReview Request: Bug 1245365 - Fix markup view search with attribute selector. r=pbro https://reviewboard.mozilla.org/r/49509/#review46399 Definitely a great start. Thanks for doing this. I have found a few things though, please take a look below. ::: devtools/client/inspector/inspector-search.js:274 (Diff revision 1) > + } > + } > + break; > + > + case this.States.ATTRIBUTE: > + if (subQuery.match(/[\[].*[\]]/) != null) { The `.*` part will catch all characters including `]` which it shouldn't, right? We want everything except the closing bracket. Should this be something like `/\[[^\]]+\]/` instead? ::: devtools/client/inspector/inspector-search.js:443 (Diff revision 1) > value = query.slice(0, -1 * lastPart.length + 1) + value; > } else if (query.match(/[a-zA-Z][#\.][^#\.\s+>]*$/)) { > // for cases like 'div.class' or '#foo.bar' and likewise > let lastPart = query.match(/[a-zA-Z][#\.][^#\.\s+>]*$/)[0]; > value = query.slice(0, -1 * lastPart.length + 1) + value; > - } else if (query.match(/[a-zA-Z]\[[^\]]*\]?$/)) { > + } else if (query.match(/[a-zA-Z]*\[[^\]]*\]\./)) { I think this misses some cases. For instances, it works fine in cases like `[id].` (finds all nodes that have an id and suggests classnames), but doesn't for `[class]#`. Which, btw, will require a new test case too. ::: devtools/client/inspector/inspector-search.js:502 (Diff revision 1) > // Hide the popup if the query ends with * because we don't want to > // suggest all nodes. This comment needs updating. We also don't want to show the popup when searching for attributes because this can possibly result in a lot of less useful results too.

https://reviewboard.mozilla.org/r/49509/#review46399 > The `.*` part will catch all characters including `]` which it shouldn't, right? We want everything except the closing bracket. > Should this be something like `/\[[^\]]+\]/` instead? You're right > I think this misses some cases. > For instances, it works fine in cases like `[id].` (finds all nodes that have an id and suggests classnames), but doesn't for `[class]#`. > > Which, btw, will require a new test case too. oh yes, I missed that. BTW, we could have [id]div too

Comment on attachment 8746640 [details] MozReview Request: Bug 1245365 - Fix markup view search with attribute selector. r=pbro Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49509/diff/1-2/

Comment on attachment 8746640 [details] MozReview Request: Bug 1245365 - Fix markup view search with attribute selector. r=pbro https://reviewboard.mozilla.org/r/49509/#review46677 Looks good to me now.

Comment on attachment 8746640 [details] MozReview Request: Bug 1245365 - Fix markup view search with attribute selector. r=pbro Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49509/diff/2-3/

TRY (https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a80e3921219) is over and everything seems fine