1363062 - Fix broken data-hidden-from-search=true search result

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

It was a mistake after applying the wrong search query selector in bug 1357352.

let rootPreferencesChildren = rootPreferences.querySelectorAll(":not([data-hidden-from-search])");

right now, the search result is weird and broken and the no result page always show empty. I'll submit a quick patch for that.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1363062 - Fix broken data-hidden-from-search=true search result

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

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8865487 [details]
Bug 1363062 - Fix broken data-hidden-from-search=true search result

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/137132/diff/1-2/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8865487 [details]
Bug 1363062 - Fix broken data-hidden-from-search=true search result

https://reviewboard.mozilla.org/r/137132/#review140628

::: browser/components/preferences/in-content/findInPage.js:205
(Diff revision 2)
> -      let rootPreferences = document.getElementById("mainPrefPane")
> -      let rootPreferencesChildren = rootPreferences
> +      let rootPreferencesChildren = document
> +        .querySelectorAll("#mainPrefPane > *:not([data-hidden-from-search])");
> -        .querySelectorAll(":not([data-hidden-from-search])");

Can you explain why the original query is broken?

Comment 4

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8865487 [details]
Bug 1363062 - Fix broken data-hidden-from-search=true search result

https://reviewboard.mozilla.org/r/137132/#review140628

> Can you explain why the original query is broken?

The original query is `let rootPreferencesChildren = rootPreferences.children;` (see https://reviewboard.mozilla.org/r/135706/diff/4#index_header)

which merely gets first level children from rootPreferences node. The `rootPreferences.querySelectorAll` will try to look through all descendant elements of rootPreferences.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8865487 [details]
Bug 1363062 - Fix broken data-hidden-from-search=true search result

https://reviewboard.mozilla.org/r/137132/#review140628

> The original query is `let rootPreferencesChildren = rootPreferences.children;` (see https://reviewboard.mozilla.org/r/135706/diff/4#index_header)
> 
> which merely gets first level children from rootPreferences node. The `rootPreferences.querySelectorAll` will try to look through all descendant elements of rootPreferences.

Ah, yes! Thanks for the explanation!

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8865487 [details]
Bug 1363062 - Fix broken data-hidden-from-search=true search result

https://reviewboard.mozilla.org/r/137132/#review140654

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ff2f063be21
Fix broken data-hidden-from-search=true search result r=mconley

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ff2f063be21

Comment 10

[Hani Yacoub, Desktop QA]

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


Verified as fixed on Firefox Nightly 55.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.