Closed
Bug 1435711
Opened 6 years ago
Closed 6 years ago
Remove controller support for tree autocomplete
Categories
(Toolkit :: Autocomplete, enhancement, P2)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: Paolo, Assigned: mak)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fxsearch])
Attachments
(1 file)
The last uses of tree autocomplete in mozilla-central are being removed in bug 1427366, so the related controller support can be removed as well.
Assignee | ||
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Updated•6 years ago
|
Whiteboard: [fxsearch]
Assignee | ||
Comment 1•6 years ago
|
||
This sounds like an interesting removal to improve the controller's maintainability
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4605aaa3e61cdea91fe2ad92f0903f0031c8041
Reporter | ||
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8952104 [details] Bug 1435711 - Remove controller support for tree autocomplete. https://reviewboard.mozilla.org/r/221330/#review227392 It's great to see the implementation of nsITreeView on the controller class going away. ::: toolkit/components/autocomplete/nsAutoCompleteController.cpp:682 (Diff revision 1) > NS_ENSURE_TRUE(result, NS_ERROR_FAILURE); > > nsAutoString search; > input->GetSearchParam(search); > > - // Clear the row in our result and in the DB. > + // Clear the march in our result and in the DB. typo: match ::: toolkit/components/autocomplete/nsAutoCompleteController.cpp (Diff revision 1) > if (aIsPopupSelection || !completeSelection) { > // We need to fill-in the value if: > // * completeselectedindex is false > - // * A row in the popup was confirmed > + // * A match in the popup was confirmed > - // > - // TODO: This is not totally correct, cause it will also confirm You know better than me why these comments are no more relevant. For curiosity, is it related to removing the support for trees, or just some drive-by cleanup?
Attachment #8952104 -
Flags: review?(paolo.mozmail) → review+
Assignee | ||
Comment 5•6 years ago
|
||
it's drive-by cleanup, we stopped selecting on mouseover.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•6 years ago
|
||
Hopefully this doesn't affect much comm-central, but since we had some misuses in m-c, it's possible those were copied around. It's mostly about using the nsiTreeview interface of the controller (like rowCount), when the same info is already available elsewhere.
Blocks: 1429115
Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/0030dd5ce5e0 Remove controller support for tree autocomplete. r=Paolo
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0030dd5ce5e0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 10•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #7) > Hopefully this doesn't affect much comm-central, ... Mildly tested, I can't see anything not working.
You need to log in
before you can comment on or make changes to this bug.
Description
•