Closed Bug 1435711 Opened 2 years ago Closed 2 years ago

Remove controller support for tree autocomplete

Categories

(Toolkit :: Autocomplete, enhancement, P2)

enhancement

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.
Priority: -- → P2
Whiteboard: [fxsearch]
This sounds like an interesting removal to improve the controller's maintainability
Assignee: nobody → mak77
Status: NEW → ASSIGNED
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+
it's drive-by cleanup, we stopped selecting on mouseover.
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
https://hg.mozilla.org/mozilla-central/rev/0030dd5ce5e0
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(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.