Closed Bug 1668147 Opened 4 years ago Closed 4 years ago

Advanced Address Book Search fails (Edit > search addresses)

Categories

(Thunderbird :: Address Book, defect)

defect

Tracking

(thunderbird_esr78 fixed, thunderbird83 fixed)

RESOLVED FIXED
84 Branch
Tracking Status
thunderbird_esr78 --- fixed
thunderbird83 --- fixed

People

(Reporter: wsmwk, Assigned: darktrojan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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

Flags: needinfo?(geoff)

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

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(geoff)
Regressed by: 1633996

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

Summary: search addresses fails in address books → Advanced Address Book Search fails (Edit > search addresses)
Attachment #9178644 - Attachment description: Bug 1668147 - Fixed advanced address book search. r?ThomasD → Bug 1668147 - Fix advanced address book search. r?ThomasD

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).

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).

Flags: needinfo?(mkmelin+mozilla)

I'm not sure what you're asking.

Flags: needinfo?(mkmelin+mozilla)

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

I'm not sure what you're asking.

Flags: needinfo?(bugzilla2007)

(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

Flags: needinfo?(bugzilla2007)
Target Milestone: --- → 84 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d255a7f17701
Fix advanced address book search. r=ThomasD

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

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.

Attachment #9178644 - Flags: approval-comm-esr78?
Attachment #9178644 - Flags: approval-comm-beta?

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

[Triage Comment]
Approved for beta

Attachment #9178644 - Flags: approval-comm-beta? → approval-comm-beta+

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

[Triage Comment]
Approved for esr78

Attachment #9178644 - Flags: approval-comm-esr78? → approval-comm-esr78+
See Also: → 1655332
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: