If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

ASSERTION: space only keyword: 'keywords.IsEmpty() || keywords.CharAt(0) != ' ''

RESOLVED FIXED

Status

MailNews Core
Backend
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Steps to reproduce problem:
1. Tag a message [keywords now "tag1"]
2. Add another tag [keywords now "tag1 tag2"]
3. Remove the second tag [keywords now "tag1 " note the extra space]
4. Add a tag again [keywords now "tag1  tag2"]
5. Remove the first tag [keywords now " tag2"]
The following conditions are required to trigger the problem:
1. There must be at least one tag present at all times
2. The first tag being removed must be the last tag to have been added
3. The second tag being removed must be the first tag to have been added
(Assignee)

Comment 1

9 years ago
Created attachment 358866 [details] [diff] [review]
Proposed patch

The first two hunks simply remove an unused variable and hoist some code out of the loop. (I guess that if you had a lot of messages then repeatedly splitting the string would waste some time.) The third hunk does the actual work; rather than deleting spaces after the keyword I decided to delete spaces before the keyword, since that was more likely to fix existing profiles. However this just reverses the problem: now deleting the first keyword will leave a leading space, so I leave a check in to delete the space after the keyword but only if it is the first one in the string. Finally, another alleged optimisation crept in, in that if no keywords were removed then there's no point setting the header. Please let me know if I've overlooked something while doing these optimisations!
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #358866 - Flags: superreview?(bienvenu)
Attachment #358866 - Flags: review?(bienvenu)
(Assignee)

Comment 2

9 years ago
Created attachment 359747 [details] [diff] [review]
nsTArray<nsCString> patch

As per bug 476150, I should have used nsTArray<nsCString>.
Attachment #358866 - Attachment is obsolete: true
Attachment #359747 - Flags: superreview?(bienvenu)
Attachment #359747 - Flags: review?(bienvenu)
Attachment #358866 - Flags: superreview?(bienvenu)
Attachment #358866 - Flags: review?(bienvenu)

Comment 3

9 years ago
Comment on attachment 359747 [details] [diff] [review]
nsTArray<nsCString> patch

looks good, thx. A couple nits:

K&R braces style has snuck in to a mostly non K&R file/function :-) 

And while you're at it, can you add a space before &length here?:

+        if (MsgFindKeyword(keywordArray[j], keywords, &startOffset,&length))
Attachment #359747 - Flags: superreview?(bienvenu)
Attachment #359747 - Flags: superreview+
Attachment #359747 - Flags: review?(bienvenu)
Attachment #359747 - Flags: review+
(Assignee)

Comment 4

9 years ago
Pushed changeset 2a5433caac6e to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.