Rename UrlbarInput::handleCommand or remove the TODO comment
Categories
(Firefox :: Address Bar, task, P3)
Tracking
()
People
(Reporter: standard8, Assigned: okonaraddi)
References
Details
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
Currently at the start of UrlbarInput::handleCommand we have:
* Handles an event which would cause a url or text to be opened.
* TODO Bug <this bug> the name is currently handleCommand which is compatible with
* urlbarBindings. However, it is no longer called automatically by autocomplete,
* See _on_keydown.
We should either rename this to something better, or remove the todo comment. Suggestions welcome.
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Gonna up the priority because P5 is almost wontfix and we should find a resolution here. Also marking Task. (By definition tasks have low user impact, and making all tasks P5s seems a bit silly.)
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Note there are some tests that call gURLBar.handleCommand(): https://searchfox.org/mozilla-central/search?q=handleCommand&case=false®exp=false&path=browser%2Fcomponents%2Furlbar
Which are right now:
- browser/components/urlbar/tests/browser/Panel.jsm
- browser/components/urlbar/tests/browser/browser_stop_pending.js
- browser/components/urlbar/tests/browser/browser_urlbarStop.js
Panel.jsm is only used by a legacy test, but the other two run on quantumbar. So it would probably be easier/better to just leave the name as handleCommand and remove the TODO.
I'm not sure what a better name would be anyway. "handleEvent" might be better since its first parameter is an event, but there's already another handleEvent method on UrlbarInput.
Assignee | ||
Comment 3•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/932c92001df0 Remove the TODO comment in UrlbarInput::handleCommand. r=adw
Comment 5•5 years ago
|
||
Comment on attachment 9069760 [details]
Bug 1536816 - Remove the TODO comment in UrlbarInput::handleCommand. r=adw!
Beta/Release Uplift Approval Request
- User impact if declined: No user impact, could potentially avoid conflicts with future quantumbar uplifts
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce: n/a
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Only removing a comment
- String changes made/needed:
Updated•5 years ago
|
Comment 6•5 years ago
|
||
bugherder |
Comment 7•5 years ago
|
||
Comment on attachment 9069760 [details]
Bug 1536816 - Remove the TODO comment in UrlbarInput::handleCommand. r=adw!
comments change, effectively a no-op, approved for 68.0b8
Comment 8•5 years ago
|
||
bugherder uplift |
Description
•