Closed Bug 406035 Opened 17 years ago Closed 17 years ago

Convert mailnews/import so it can compile with frozen api on linux

Categories

(MailNews Core :: Import, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch The fix (obsolete) — Splinter Review
The patch I'm attaching moves the linux parts of mailnews/import so that they can be compiled with the frozen api ready for libxul (this leaves the outlook & eudora import items). We can't link it with the frozen api yet because the core mailnews code on which it depends still needs converting. There may be one other link problem, but I'm just looking at getting the majority of it converted. However, this is another step in the right direction.
Attachment #290706 - Flags: superreview?(neil)
Attachment #290706 - Flags: review?(neil)
Comment on attachment 290706 [details] [diff] [review] The fix >+#ifdef MOZILLA_INTERNAL_API > offset = buffer.Find(prefName,PR_FALSE, 0, -1); >+#else >+ offset = buffer.Find(prefName, PR_FALSE); >+#endif Under external API, the second parameter to Find is not a PRBool, but as it happens you want the default values for all optional parameters anyway, so: offset = buffer.Find(prefName); should suffice, shouldn't it? Similarly for the other uses of Find. >- field.ReplaceSubstring( "\"\"", "\""); >+ PRInt32 offset = field.Find(NS_LITERAL_CSTRING("\"\"")); >+ while (offset != -1) { >+ field.Replace(offset, 2, "\""); >+ } You don't Find again after your Replace (also below). Also you can in theory write your Replace differently to just delete one of the two quotes. > if ((idx != -1) && (idx > 0) && ((name.Length() - idx - 1) < 5)) { >- nsString t; >- name.Left( t, idx); >- name = t; >+ name = StringHead(name, idx); > } Would name.SetLength(idx); work?
Attachment #290706 - Flags: superreview?(neil)
Attachment #290706 - Flags: superreview-
Attachment #290706 - Flags: review?(neil)
Attached patch The fix v2Splinter Review
Addressed Neil's comments.
Attachment #290706 - Attachment is obsolete: true
Attachment #290866 - Flags: superreview?(neil)
Attachment #290866 - Flags: review?(neil)
Comment on attachment 290866 [details] [diff] [review] The fix v2 >+ if (endOffset != -1) { >+ nsAutoString prefValue(Substring(buffer, offset + PREF_LENGTH, endOffset - (offset + PREF_LENGTH))); >+ found = PR_TRUE; >+ *retval = ToNewUnicode(prefValue); >+ break; > } I notice you can call ToNewUnicode directly on a Substring, no need to copy it to an nsAutoString first. It also looks as if you forgot to indent this block. >+ PRInt32 offset = field.Find(NS_LITERAL_CSTRING("\"\"")); >+ while (offset != -1) { >+ field.Replace(offset, 1, EmptyCString()); >+ offset = field.Find(NS_LITERAL_CSTRING("\"\"")); >+ } This won't work correctly if the field contains multiple consecutive quotes - because you don't start your match after the previous quote you can keep matching the same quote over and again. r+sr=me with this fixed. You can also use Cut instead of Replace. (The loops later on don't have this bug because they completely remove the search string, but the change would still optimise them).
Attachment #290866 - Flags: superreview?(neil)
Attachment #290866 - Flags: superreview+
Attachment #290866 - Flags: review?(neil)
Attachment #290866 - Flags: review+
I checked this in earlier with Neil's comments addressed.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: