1668147 - Advanced Address Book Search fails (Edit > search addresses)

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Wayne Mery (:wsmwk)]

I'm using beta 82.0b1, but I assume 78 also fails (I could be wrong)

steps: address book > edit search addresses > pick any(?) field, for example pager

results: no results listed

NS_ERROR_MALFORMED_URI: Unexpected uri: jsaddrbook://abook-4.sqlite?(and(WorkPhone,c,employee)) AddrBookManager.jsm:205
getDirectory resource:///modules/AddrBookManager.jsm:205
GetDirectoryFromURI chrome://messenger/content/addressbook/abCommon.js:928
SetAbView chrome://messenger/content/addressbook/abResultsPane.js:81
onSearch chrome://messenger/content/addressbook/abSearchDialog.js:337
onSearchButton chrome://messenger/content/addressbook/abSearchDialog.js:343
oncommand chrome://messenger/content/addressbook/abSearchDialog.xhtml:1
openWindowPrompt resource:///actors/PromptParent.jsm:160
receiveMessage resource:///actors/PromptParent.jsm:108
openPrompt resource://gre/modules/Prompter.jsm:1182
openPromptSync resource://gre/modules/Prompter.jsm:1070
confirmEx resource://gre/modules/Prompter.jsm:1447

Comment 1

[Geoff Lankow (:darktrojan)]

Yeah, I was vaguely aware of this. It's also the problem holding up bug 1658131. That dialog has zero tests.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1668147 - Fix advanced address book search. r?ThomasD

This change should've been made with bug 1633620 or bug 1633996, but wasn't.

Comment 3

[Thomas D. (:thomas8)]

I must admit I thoroughly detest the deep internals of the address book - I really feel for Geoff because the way things are nested in there is just mind-warping! Our current construct for "All Address Books" looks pretty fragile: returning directory = null for (!uri) but also for (uri==kAllDirectoryRoot aka "moz-abdirectory://") where we really mean "All Address Books" looks like a recipe for errors.
Anyway, I've tried my best to cut through the jungle in my detailed review on phabricator.

https://phabricator.services.mozilla.com/D91884#2998738 (you'll have to scroll down for the yellow comment, as phabricator jumps back to the top after selecting that).

Comment 4

[Thomas D. (:thomas8)]

Fyi

(In reply to Thomas D. (:thomas8) from comment #3)

I must admit I thoroughly detest the deep internals of the address book - I really feel for Geoff because the way things are nested in there is just mind-warping! Our current construct for "All Address Books" looks pretty fragile: returning directory = null for (!uri) but also for (uri==kAllDirectoryRoot aka "moz-abdirectory://") where we really mean "All Address Books" looks like a recipe for errors.
Anyway, I've tried my best to cut through the jungle in my detailed review on phabricator.

https://phabricator.services.mozilla.com/D91884#2998738 (you'll have to scroll down for the yellow comment, as phabricator jumps back to the top after selecting that).

Comment 5

[Magnus Melin [:mkmelin]]

I'm not sure what you're asking.

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #5)

I'm not sure what you're asking.

Comment 7

[Thomas D. (:thomas8)]

(In reply to Magnus Melin [:mkmelin] from comment #5)

I'm not sure what you're asking.

Ah sorry, no question here, just "for your interest" (fyi) as a pointer to my deep review which I've spent time on:
https://phabricator.services.mozilla.com/D91884#2998738

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d255a7f17701
Fix advanced address book search. r=ThomasD

Comment 10

[Geoff Lankow (:darktrojan)]

Comment on attachment 9178644 [details]
Bug 1668147 - Fix advanced address book search. r?ThomasD

[Approval Request Comment]
Regression caused by (bug #): Many address book changes
User impact if declined: Advanced AB search doesn't work at all
Testing completed (on c-c, etc.): On CC since last week
Risk to taking this patch (and alternatives if risky): Doesn't change anything that isn't broken already, so no real risk.

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9178644 [details]
Bug 1668147 - Fix advanced address book search. r?ThomasD

[Triage Comment]
Approved for beta

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 83.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/db8cbe2b7c97

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9178644 [details]
Bug 1668147 - Fix advanced address book search. r?ThomasD

[Triage Comment]
Approved for esr78

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 78.4.1:
https://hg.mozilla.org/releases/comm-esr78/rev/75f47f3ed645