2064279 - Gate address-bar-only chrome machinery on the input being the address bar

Status: REOPENED

(Firefox :: Address Bar, task)

Description

[Dão Gottwald [:dao]]

Two pieces of UrlbarInputBase reach for chrome window structure that only the address bar needs, which a content-realm <moz-urlbar> then trips over.

_initCopyCutController() installs a controller so copy/cut put an untrimmed URL on the clipboard. _trimValue() returns early unless the input is the address bar, so on the searchbar the controller only reimplements native copy/cut -- and its hand-rolled cmd_cut splices inputField.value directly instead of going through the editor, so it isn't undoable. It needs controllers and editor on the input plus the clipboard service, all chrome-only.

#deferUpdatePlaceholder() defers the placeholder update until the user looks away, taking TabSelect on gBrowser.tabContainer as one of its two cues. Only the address bar takes its value from the selected tab, so on the searchbar that listener can only ever fire with its this.value guard already false, by which time the input listener has removed both.

Gating both on #isAddressbar drops dead work from the searchbar and lets a content instance work without a realm branch. Note the small behaviour change for the chrome searchbar: cmd_cut goes back through the editor, restoring undo.

addGBrowserListeners() should keep its this.window.gBrowser check rather than move to an #isAddressbar gate -- its TabClose handler triggers bounce telemetry, which is recorded for the searchbar SAP too.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 2064279 - Gate the copy/cut controller on the address bar. r?mbeier!

It exists to put an untrimmed URL on the clipboard, and _trimValue returns
early unless the input is the address bar, so on the searchbar the controller
only reimplements native copy/cut -- and its hand-rolled cmd_cut splices
inputField.value directly, so it doesn't go through the editor's undo.

This is what lets a content-realm input work without a realm branch here: the
controller needs controllers and editor on the input plus the clipboard
service, all chrome-only.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 2064279 - Defer the placeholder update to a tab switch only in the address bar. r?mbeier!

The update waits for the user to look away, and a tab switch qualifies only for
the address bar, which takes its value from the selected tab. Nothing gives the
searchbar a value on TabSelect, so its listener could only ever fire with the
guard already false -- and by then the input listener has removed both.

A content-realm input, not being the address bar, is left with the same single
cue, which is what this needs to not reach for a tabbrowser it doesn't have.

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 2064279 - Gate the search-terms persistence check on the address bar. r?mbeier!

Persistence is an address bar feature and its state is keyed by the selected
tab's browser, which a content-realm input has no way to reach.

Worth knowing how this presented: handleEvent wraps every _on_* in a
try/catch and reports through console.error, so the resulting exception left
_on_input silently half-done -- typing updated the value and started no query,
with nothing in the parent-process log.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/aa895826ce3d
https://hg.mozilla.org/integration/autoland/rev/bdb2fd01d5f2
Gate the copy/cut controller on the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/e0baa9504fa5
https://hg.mozilla.org/integration/autoland/rev/484b10e80c66
Defer the placeholder update to a tab switch only in the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/69cdce10fed3
https://hg.mozilla.org/integration/autoland/rev/6176008fb1e9
Gate the search-terms persistence check on the address bar. r=mbeier,urlbar-reviewers

Comment 5

[Serban Stanca [:SerbanS]]

Reverted this because it was causing mochitests failures in browser_searchbar_new.js.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | accessible/tests/browser/tree/browser_searchbar_new.js | test_searchbar_a11y_tree - [DOM node id: searchbar-new, role: grouping, address: [xpconnect wrapped nsIAccessible]] has an extra child at index 1 : [role: section, address: [xpconnect wrapped nsIAccessible]]

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4ef9eb707145
https://hg.mozilla.org/integration/autoland/rev/58018de9cba7
Gate the copy/cut controller on the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/cc877b194274
https://hg.mozilla.org/integration/autoland/rev/d597e8bc5bc4
Defer the placeholder update to a tab switch only in the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/85641c7382b3
https://hg.mozilla.org/integration/autoland/rev/2badfba80ef9
Gate the search-terms persistence check on the address bar. r=mbeier,urlbar-reviewers

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/bdb2fd01d5f2
https://hg.mozilla.org/mozilla-central/rev/484b10e80c66
https://hg.mozilla.org/mozilla-central/rev/6176008fb1e9

Comment 8

[Dão Gottwald [:dao]]

Attachment: Bug 2064279 - Keep per-tab state and the trimmed-prefix derivation to the address bar. r?daleharvey!

The unified search button's availability is stored per tab, which only a browser window has, so a content realm threw on every result selection. _trimValue only trims in the address bar, so elsewhere there is no prefix to derive and no need for BrowserUIUtils.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2422c061baca
https://hg.mozilla.org/integration/autoland/rev/8df4863f6ccd
Gate the copy/cut controller on the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/7cdffe1b72d1
https://hg.mozilla.org/integration/autoland/rev/3b1a89228845
Defer the placeholder update to a tab switch only in the address bar. r=mbeier,urlbar-reviewers
https://github.com/mozilla-firefox/firefox/commit/49548db7628d
https://hg.mozilla.org/integration/autoland/rev/9400c5c7ca2a
Gate the search-terms persistence check on the address bar. r=mbeier,urlbar-reviewers

Comment 10

[BugBot [:suhaib / :marco]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)