1956634 - Enable <input type="search"> in privileged documents

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, task)

Description

[Tim Giles [:tgiles]]

By enabling the <input type="search"> clear search button in privileged documents, regardless of the layout.forms.input-type-search.enabled pref, would be beneficial for the following reasons:

• It enables the moz-input-search component to be utilized in our UI without needing to create a duplicate implementation of the "clear search" functionality
• It allows us to consolidate and merge duplicate "search box with clear button" implementations:
  • Firefox View
    • Changing this one should also affect the sidebar, since the "Tabs from other devices" and "History" sections also use this particular search textbox.
  • DevTools
  • GeckoView
• It increases visibility of the "clear search" button and any user experience issues with said input without impacting the entire web
• It moves us closer to being able to turn on layout.forms.input-type-search.enabled by default (Bug 1654288)

We will need to address the previous cases before enabling the "clear search" button, otherwise each of the cases will have duplicate buttons that are styled differently.

Comment 1

[Tim Giles [:tgiles]]

Attachment: Bug 1956634 - Enable clear search button in chrome documents. r=emilio,#recomp-reviewers

Since this will result in duplicate UI elements in various places,
we should wait until the start of a new Nightly cycle to land the
change. See Bug 1936920, Bug 1956650, and Bug 1956812 for known places
where there will be duplicate clear search buttons.

Depends on D243387

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae2567f945c9
Enable clear search button in chrome documents. r=emilio,reusable-components-reviewers,mstriemer

Comment 3

[Sandor Molnar[:smolnar]]

Backed out for causing bc failures @ browser_history_sidebar.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/2db065807374c10d5bbaab5640c01bfb8c5d11e1

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | browser/components/sidebar/tests/browser/browser_history_sidebar.js | Uncaught exception in test bound test_history_search

Comment 4

[Atila Butkovits]

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

Comment 5

[Tim Giles [:tgiles]]

Hey Emilio, following up on my request for some help debugging a devtools test failure that seems related to min-width calculations or some other related issue. The particular test I'm trying to fix is browser_webconsole_filter_buttons_overflow.js which passes without issue on central. However, once I introduce the "enable clear search button" patch, then the test fails because the layout of the webconsole view doesn't switch from "wide" to "narrow". Even if I remove (or use display: none) the previously existing button, I can't get the devtools webconsole view to wrap the search/filter input as expected.

You can easily reproduce this issue by setting layout.forms.input-type-search.enabled=true in the _webconsole.toml file at line 195.

Not sure what else to look into to figure out this issue and would appreciate your insight into this. Thanks!

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1956634 - nsSearchControlFrame should be a replaced frame. r=#layout,tgiles

Not just a leaf. This matches nsNumberControlFrame and co.

This goes all the way back to bug 558594 and fixes the test failures
described in comment 5.

I'll check if there's any WPT covering this.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Seems like a bug in the search control frame implementation!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18e2ccc21035
nsSearchControlFrame should be a replaced frame. r=layout-reviewers,jfkthame

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/18e2ccc21035

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Err sorry thought you were going to land the actual enablement while at it. Probably should've landed it in a separate bug.

Comment 11

[Tim Giles [:tgiles]]

Attachment: Bug 1956634 - Fix sidebar tests when clear search button is enabled. r=#sidebar-reviewers

In order to get this stack of patches to land, I needed to hide the
extra clear search button that is in fxview-search-textbox.

I needed to update browser_history_sidebar to access the anonymous
content element, the clear search button, in order to keep the test
as designed.

I needed to update browser_adopt_sidebar_from_opener due to the sidebar
width not being reset to the original value after running this test.
This narrow width would cause the next ran test to fail in non-obvious
ways.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8b5daf5bd4b8
https://hg.mozilla.org/integration/autoland/rev/03909323faaa
Enable clear search button in chrome documents. r=emilio,reusable-components-reviewers,mstriemer
https://github.com/mozilla-firefox/firefox/commit/d50510a19dfc
https://hg.mozilla.org/integration/autoland/rev/5618a401d844
Fix sidebar tests when clear search button is enabled. r=sidebar-reviewers,fxview-reviewers,jsudiaman

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6d8526aea99a
https://hg.mozilla.org/integration/autoland/rev/8c40a5486452
Revert "Bug 1956634 - Fix sidebar tests when clear search button is enabled. r=sidebar-reviewers,fxview-reviewers,jsudiaman" for causing bc failures @ browser_history_firefoxview.js

Comment 14

[amarc]

Backed out for causing bc fails @ browser_history_firefoxview.js

• Backout link
• Push with failures
• Failure Log

Comment 15

[Tim Giles [:tgiles]]

Attachment: Bug 1956634 - Fix Firefox View tests when HTML clear search button is enabled. r=#fxview-reviewers

Since there isn't a separate fxview clear search button, the tests
needed to be updated to activate the clear search button that exists
in the HTMLInputElement.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ee0ab707db45
https://hg.mozilla.org/integration/autoland/rev/9fb1257fe4fe
Enable clear search button in chrome documents. r=emilio,reusable-components-reviewers,mstriemer
https://github.com/mozilla-firefox/firefox/commit/fa3f818fab75
https://hg.mozilla.org/integration/autoland/rev/20805a0ce840
Fix sidebar tests when clear search button is enabled. r=sidebar-reviewers,fxview-reviewers,jsudiaman
https://github.com/mozilla-firefox/firefox/commit/52faf8c4c453
https://hg.mozilla.org/integration/autoland/rev/765d3f515c66
Fix Firefox View tests when HTML clear search button is enabled. r=fxview-reviewers,nsharpley

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e3e168c03735
https://hg.mozilla.org/integration/autoland/rev/fc9986230203
Revert "Bug 1956634 - Fix Firefox View tests when HTML clear search button is enabled. r=fxview-reviewers,nsharpley" for causing bc fails @ browser_history_firefoxview.js & browser_history_sidebar.js

Comment 18

[Sandor Molnar[:smolnar]]

Backed out for causing bc fails @ browser_history_firefoxview.js & browser_history_sidebar.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/fc99862302036c47353c398bb6a925b54962220a

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | browser/components/sidebar/tests/browser/browser_history_sidebar.js | Uncaught exception in test bound test_history_search

Failure log -> TEST-UNEXPECTED-FAIL | browser/components/firefoxview/tests/browser/browser_history_firefoxview.js | Uncaught exception in test bound test_search_history

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/bc3b83054076
https://hg.mozilla.org/integration/autoland/rev/1cdd656f5a5e
Enable clear search button in chrome documents. r=emilio,reusable-components-reviewers,mstriemer
https://github.com/mozilla-firefox/firefox/commit/40a534a682de
https://hg.mozilla.org/integration/autoland/rev/a2e9312df4d8
Fix sidebar tests when clear search button is enabled. r=sidebar-reviewers,fxview-reviewers,jsudiaman
https://github.com/mozilla-firefox/firefox/commit/f787e3413bde
https://hg.mozilla.org/integration/autoland/rev/d8971955d303
Fix Firefox View tests when HTML clear search button is enabled. r=fxview-reviewers,nsharpley

Comment 20

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/1cdd656f5a5e
https://hg.mozilla.org/mozilla-central/rev/a2e9312df4d8
https://hg.mozilla.org/mozilla-central/rev/d8971955d303