Drop nsIAbMDBDirectory::getDirUri

RESOLVED FIXED

Status

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Created attachment 312877 [details] [diff] [review]
The fix

nsIAbMDBDirectory::getDirUri is the same as getting the URI from the RDF resource. Its only used in one place which can easily be changed.

I've also changed a function call in nsMsgCompose (as I wanted to remove the comment) to avoid a getter_Copies call.

I want this to help start to clean up our current mess of get URI implementations.
Attachment #312877 - Flags: superreview?(neil)
Attachment #312877 - Flags: review?(neil)
(Assignee)

Comment 1

11 years ago
Created attachment 312945 [details] [diff] [review]
The fix v2

Improved version removing the rdf api calls.
Attachment #312877 - Attachment is obsolete: true
Attachment #312945 - Flags: superreview?(neil)
Attachment #312945 - Flags: review?(neil)
Attachment #312877 - Flags: superreview?(neil)
Attachment #312877 - Flags: review?(neil)

Comment 2

11 years ago
Comment on attachment 312945 [details] [diff] [review]
The fix v2

I particularly like the nsMsgCompose change :-)
Attachment #312945 - Flags: superreview?(neil)
Attachment #312945 - Flags: superreview+
Attachment #312945 - Flags: review?(neil)
Attachment #312945 - Flags: review+
(Assignee)

Comment 3

11 years ago
Patch checked in -> fixed.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 4

11 years ago
While reviewing the address book search setup as part of preparing unit tests for bug 187768, I saw some code I did not understand which I traced to this patch.

Previously, in nsMsgSearchTerm::InitializeAddressBook there was this code:

    if (strcmp(dirURI.get(), m_value.string))
      mDirectory = nsnull; // clear out the directory....we are no longer pointing to the right one


which has now been replaced by:

    if (uri.Equals(m_value.string))
      // clear out the directory....we are no longer pointing to the right one
      mDirectory = nsnull;

But the logic is backwards. strcmp returns 0 (false) when the string are equal. So the new code should be:

    if (!uri.Equals(m_value.string))
(Assignee)

Updated

11 years ago
Depends on: 436104
(Assignee)

Comment 5

11 years ago
(In reply to comment #4)
> While reviewing the address book search setup as part of preparing unit tests
> for bug 187768, I saw some code I did not understand which I traced to this
> patch.

As this was code that got released (TB 3.0a1), I've raised a separate bug to handle the fixing of this (bug 436104).
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.