1980892 - Inspector search can't find css selector with hyphen

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Julian Descottes [:jdescottes]]

STRs:

• open data:text/html,<div class="a b-c">
• open inspector
• try to search div.a -> OK, .b-c -> OK, div.b-c -> KO

Comment 1

[Julian Descottes [:jdescottes]]

See original report https://discourse.mozilla.org/t/why-does-div-no-data-work-in-chrome-devtools-but-not-in-firefox-devtools-when-using-the-ctrl-f-search-bar/145335

Comment 2

[Julian Descottes [:jdescottes]]

Note that the popup suggests div.b-c in the autocomplete.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

That's because of https://searchfox.org/mozilla-central/rev/81221d27112f12a67cb86287bf2b3cd9f19373af/devtools/server/actors/utils/walker-search.js#265,270-272,275-282

_searchSelectors(query, options, results) {
...
  // If the query is just one "word", no need to search because _searchIndex
  // will lead the same results since it has access to tagnames anyway
  if (
...
    // custom element names
    InspectorUtils.isCustomElementName(
      query,
      this.walker.targetActor.window.document.documentElement.namespaceURI
    )
  ) {
    return;
  }

InspectorUtils.isCustomElementName("div.b-c") returns true (so does InspectorUtils.isCustomElementName("div#b-c"))

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1980892 - [devtools] Remove use of InspectorUtils.isCustomElementName on WalkerSearch#_searchSelectors. r=#devtools.

We used to check if the passed query looked like a tag name so we could
bail out early, as we'd get those results from _searchIndex.
But InspectorUtils.isCustomElementName does match some legitimate
selectors, like div.a-b.
Let's remove the check on custom element and trigger the search in such case.

Comment 5

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

Set release status flags based on info from the regressing bug 1871881

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/984841a1f95e
https://hg.mozilla.org/integration/autoland/rev/a32bf35caeca
[devtools] Remove use of InspectorUtils.isCustomElementName on WalkerSearch#_searchSelectors. r=devtools-reviewers,jdescottes.

Comment 7

[Cristina Horotan [:chorotan]]

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

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1980892 - [devtools] Remove use of InspectorUtils.isCustomElementName on WalkerSearch#_searchSelectors. r=#devtools.

We used to check if the passed query looked like a tag name so we could
bail out early, as we'd get those results from _searchIndex.
But InspectorUtils.isCustomElementName does match some legitimate
selectors, like div.a-b.
Let's remove the check on custom element and trigger the search in such case.

Original Revision: https://phabricator.services.mozilla.com/D259783

Comment 9

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Searching for an element in the inspector using a selector might not work
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: low
• Explanation of risk level: devtools only patch, removing a simple guard, covered by mochitest
• String changes made/needed: -
• Is Android affected?: no

Comment 10

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/aeb9e7ff44cb
https://hg.mozilla.org/releases/mozilla-beta/rev/b699821f2127

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1980892 - [devtools] Remove use of InspectorUtils.isCustomElementName on WalkerSearch#_searchSelectors. r=#devtools.

We used to check if the passed query looked like a tag name so we could
bail out early, as we'd get those results from _searchIndex.
But InspectorUtils.isCustomElementName does match some legitimate
selectors, like div.a-b.
Let's remove the check on custom element and trigger the search in such case.

Original Revision: https://phabricator.services.mozilla.com/D259783

Comment 12

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Searching for an element in the inspector using a selector might not work
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: low
• Explanation of risk level: devtools only patch, removing a simple guard, covered by mochitest
• String changes made/needed: -
• Is Android affected?: no

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/591adea10878