Mark Banner (:standard8) Assignee
	Description • 17 years ago

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.

	neil@parkwaycc.co.uk
	Comment 1 • 17 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?

	Mark Banner (:standard8) Assignee
	Comment 2 • 17 years ago

Addressed Neil's comments.

	neil@parkwaycc.co.uk
	Comment 3 • 17 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).

	Mark Banner (:standard8) Assignee
	Comment 4 • 17 years ago

I checked this in earlier with Neil's comments addressed.