Remove the remaining bare ChromeUtils uses in urlbar content code
Categories
(Firefox :: Address Bar, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox156 | --- | fixed |
People
(Reporter: dao, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [sng][mcab-hnt])
Attachments
(3 files)
Bug 2053780 and bug 2064036 removed the lazy getters that ChromeUtils and XPCOMUtils defined in browser/components/urlbar/content/, and bug 2062987 gates the system module imports on ChromeUtils being available. What neither covers is a handful of direct ChromeUtils calls, each of which throws in a content realm where ChromeUtils is undefined:
ChromeUtils.generateQI--SearchModeSwitcher.mjs(in the constructor, so it runs for every switcher, content included) andUrlbarInputBase.mjs, both to implementnsIObserver/nsISupportsWeakReference.ChromeUtils.encodeURIForSrcset--UrlbarInputBase.mjsandSmartbarInput.mjs, both when setting an engine icon.ChromeUtils.now--UrlbarChildTelemetry.mjs, which is the content-side engagement collector used on the message path.
At least the SearchModeSwitcher constructor and UrlbarChildTelemetry sit on paths a content-realm <moz-urlbar> reaches, so these are functional blockers rather than tidiness.
Notes toward fixes: generateQI is there because these objects are XPCOM observers, so if the observer registration moves to the parent the QueryInterface goes with it. ChromeUtils.now has performance.now() as a content-safe equivalent, modulo the time base. encodeURIForSrcset has no content-realm equivalent and would need the actor, or a different way to set the icon.
|
Assignee | |
Updated•2 days ago
|
|
||
Updated•2 days ago
|
|
|
Assignee | |
Comment 1•2 days ago
|
||
SearchModeSwitcher's QueryInterface is dead -- nothing registers it with the
observer service anymore, and Cu.getWeakReference, which is what
UrlbarPrefs.addObserver uses, works on a plain object. UrlbarInputBase's
observer was a field initializer, so it ran generateQI for every input;
building it where it's registered keeps it in the branch that needs the observer
service anyway.
The telemetry event.timeStamp || ChromeUtils.now() fallback is unreachable:
every caller passes a DOM event, and even the undispatched CustomEvent the
tab-switch path constructs carries a timeStamp. Both copies drop it. It read from
a different clock than event.timeStamp does, so it would have been off by the
window's time origin had it ever fired.
The two encodeURIForSrcset calls stay: they build XUL, so a content realm never
reaches them.
|
|
Assignee | |
Comment 2•1 day ago
|
||
observe matches the notification's subject against the input's own window, so a
content-realm input could never match it -- and generateQI and the observer
service are chrome-only, so reaching for them threw before the engine-store
observer below could be registered. That one a content realm does need.
|
|
Assignee | |
Comment 3•1 day ago
|
||
UrlbarQueryContext's constructor normalizes the container id, so this sits on
the path of every query -- a content-realm input couldn't run one at all. The
value is spelled out beside the private-window sentinel the same file already
carries.
|
||
Comment 6•14 hours ago
|
||
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 8•12 hours ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/df9ba132e2a0
https://hg.mozilla.org/mozilla-central/rev/25d6d3d69f82
https://hg.mozilla.org/mozilla-central/rev/3c753113721b
|
||
Comment 10•1 hour ago
|
||
| bugherder | ||
Description
•