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

RESOLVED FIXED

Status

MailNews Core
Import
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

19.08 KB, patch
neil@parkwaycc.co.uk
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

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

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 1

11 years ago
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)
(Assignee)

Comment 2

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

Addressed Neil's comments.
Attachment #290706 - Attachment is obsolete: true
Attachment #290866 - Flags: superreview?(neil)
Attachment #290866 - Flags: review?(neil)

Comment 3

11 years ago
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+
(Assignee)

Comment 4

11 years ago
I checked this in earlier with Neil's comments addressed.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.