Make mailnews/mime compile and link capable with frozen linkage

RESOLVED FIXED

Status

MailNews Core
MIME
P3
normal
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Assignee)

Description

11 years ago
Created attachment 294816 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 1

As part of the ongoing effort, mailnews/mime needs moving to frozen linkage.

I'm going to do this in two parts (well maybe three), first to do the simple "mechanical" changes in mailnews/mime/src, second to do the more complicated changes there. Then I'll do the emitters and cthandler parts.

So attaching the first part - mainly header include changes. One proxy change as we've done elsewhere.
Attachment #294816 - Flags: superreview?(bienvenu)
Attachment #294816 - Flags: review?(bienvenu)

Comment 1

11 years ago
Comment on attachment 294816 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 1

thx, Mark.
Attachment #294816 - Flags: superreview?(bienvenu)
Attachment #294816 - Flags: superreview+
Attachment #294816 - Flags: review?(bienvenu)
Attachment #294816 - Flags: review+
(Assignee)

Updated

11 years ago
Attachment #294816 - Attachment description: Migrate mailnews/mime to frozen linkage part 1 → [checked in] Migrate mailnews/mime to frozen linkage part 1
(Assignee)

Comment 2

11 years ago
Created attachment 294903 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 2

This does the rest of migrating mailnews/mime/src to frozen linkage apart from one item that will fail at link time - I'm still considering what to do with that.

Note that the do_CreateInstanceFromCategory replacement is the only place that function is used, and so I'll file a follow up bug after commiting this to remove it from core code.
Attachment #294903 - Flags: review?(bienvenu)

Comment 3

11 years ago
Comment on attachment 294903 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 2

ugh, libmime - thx for taking this on, Mark.
Attachment #294903 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

11 years ago
Attachment #294903 - Flags: superreview?(neil)

Comment 4

11 years ago
Comment on attachment 294903 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 2

>+  PRInt32 pos = orig_url.Find("mailbox-message");
>+  if (pos != -1)
>+    orig_url.Replace(pos, 15, "mailbox", 7);
Could be orig_url.Cut(pos + 8, 7); perhaps?

>-      rv = conv->ScanTXT(citeTagsSource.get(), 0 /* no recognition */,
>-                         getter_Copies(citeTagsResultUnichar));
>+      rv = conv->ScanTXT(citeTagsSource.get(),
>+                         0 /* no recognition */, getter_Copies(citeTagsResultUnichar));
What's the change here? As far as I can see all you do is use up columns.

>+#ifdef MOZILLA_INTERNAL_API
>       if (mRealContentType.LowerCaseEqualsLiteral("message/rfc822"))
>+#else
>+      if (mRealContentType.Equals("message/rfc822", CaseInsensitiveCompare))
>+#endif
Eww, external API doesn't do LowerCaseEqualsLiteral on nsCString?
But didn't I see somewhere that we're now lowercasing content types?
Attachment #294903 - Flags: superreview?(neil) → superreview+

Comment 5

11 years ago
(In reply to comment #4)
>Eww, external API doesn't do LowerCaseEqualsLiteral on nsCString?
Double Eww, it gives different answers on nsString to internal API...
(Assignee)

Comment 6

11 years ago
(In reply to comment #4)
> (From update of attachment 294903 [details] [diff] [review])
> >+  PRInt32 pos = orig_url.Find("mailbox-message");
> >+  if (pos != -1)
> >+    orig_url.Replace(pos, 15, "mailbox", 7);
> Could be orig_url.Cut(pos + 8, 7); perhaps?

I've changed this, but to orig_url.Cut(pos + 7, 8);

> >-      rv = conv->ScanTXT(citeTagsSource.get(), 0 /* no recognition */,
> >-                         getter_Copies(citeTagsResultUnichar));
> >+      rv = conv->ScanTXT(citeTagsSource.get(),
> >+                         0 /* no recognition */, getter_Copies(citeTagsResultUnichar));
> What's the change here? As far as I can see all you do is use up columns.

No idea, I dropped it.

> >+#ifdef MOZILLA_INTERNAL_API
> >       if (mRealContentType.LowerCaseEqualsLiteral("message/rfc822"))
> >+#else
> >+      if (mRealContentType.Equals("message/rfc822", CaseInsensitiveCompare))
> >+#endif
> Eww, external API doesn't do LowerCaseEqualsLiteral on nsCString?
> But didn't I see somewhere that we're now lowercasing content types?

Yes, see bug 409962. I've changed this whole section to just:

if (mRealContentType.Equals("message/rfc822")

and done the same for the next two bits as well. I loaded a message with an "Message/Rfc822" attachment and could still view and open it fine. So I believe this shouldn't regress bug 409962.

Checked in with those changes.
(Assignee)

Updated

11 years ago
Attachment #294903 - Attachment description: Migrate mailnews/mime to frozen linkage part 2 → [checked in] Migrate mailnews/mime to frozen linkage part 2
(Assignee)

Comment 7

11 years ago
Created attachment 294959 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 3

This does most of the rest of mailnews/mime (cthandlers and emitters). Note that it appears that cthandlers/smimestub hasn't been built for a while as it doesn't actually link (due to incorrect lib location) and would need someone building with --disable-crpyto, but I fixed it anyway.

After we've fixed this, there's only about 2 or 3 other functions in mailnews/mime to have a frozen api solution implemented, but they are a bit more complicated and I'm still working out the best solutions.
Attachment #294959 - Flags: superreview?(bienvenu)
Attachment #294959 - Flags: review?(bienvenu)
(Assignee)

Updated

11 years ago
Depends on: 410332

Updated

11 years ago
Attachment #294959 - Flags: superreview?(bienvenu)
Attachment #294959 - Flags: superreview+
Attachment #294959 - Flags: review?(bienvenu)
Attachment #294959 - Flags: review+
(Assignee)

Comment 8

11 years ago
Comment on attachment 294959 [details] [diff] [review]
[checked in] Migrate mailnews/mime to frozen linkage part 3

Checked in with one minor bustage fix (I had to add an nsEscape.h include back in, guess I'd cut too much out in forming the patch).
Attachment #294959 - Attachment description: Migrate mailnews/mime to frozen linkage part 3 → [checked in] Migrate mailnews/mime to frozen linkage part 3
(Assignee)

Comment 9

11 years ago
Created attachment 302442 [details] [diff] [review]
[checked in] Fix Thunderbird-specific mimevcrd.cpp code

Simple change to fix TB specific parts of mimevcrd.cpp (the bit that caused me to put nsEscape.h include back in before).
Attachment #302442 - Flags: superreview?(bienvenu)
Attachment #302442 - Flags: review?(bienvenu)

Comment 10

11 years ago
Comment on attachment 302442 [details] [diff] [review]
[checked in] Fix Thunderbird-specific mimevcrd.cpp code

thx, Mark.
Attachment #302442 - Flags: superreview?(bienvenu)
Attachment #302442 - Flags: superreview+
Attachment #302442 - Flags: review?(bienvenu)
Attachment #302442 - Flags: review+
(Assignee)

Updated

11 years ago
Attachment #302442 - Attachment description: Fix Thunderbird-specific mimevcrd.cpp code → [checked in] Fix Thunderbird-specific mimevcrd.cpp code
(Assignee)

Comment 11

10 years ago
Created attachment 321767 [details] [diff] [review]
[checked in] More include fixes changes

More include fixes following the base/utils change.
Attachment #321767 - Flags: superreview?(neil)
Attachment #321767 - Flags: review?(neil)

Comment 12

10 years ago
Comment on attachment 321767 [details] [diff] [review]
[checked in] More include fixes changes

>diff -r 970dc0914d0c mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp
>--- a/mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp	Mon May 19 14:49:15 2008 +0100
>+++ b/mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp	Mon May 19 14:54:17 2008 +0100
>@@ -62,6 +62,7 @@
> #include "nsDateTimeFormatCID.h"
> #include "nsMsgUtils.h"
> #include "nsINetUtil.h"
>+#include "nsMemory.h"
Side note: aren't we supposed to be switching over to NS_Alloc/Free directly?
Attachment #321767 - Flags: superreview?(neil)
Attachment #321767 - Flags: superreview+
Attachment #321767 - Flags: review?(neil)
Attachment #321767 - Flags: review+
(Assignee)

Updated

10 years ago
Attachment #321767 - Attachment description: More include fixes changes → [checked in] More include fixes changes
(Assignee)

Updated

10 years ago
Priority: -- → P3
Product: Core → MailNews Core
(Assignee)

Comment 13

9 years ago
Created attachment 369520 [details] [diff] [review]
[checked in] Remaining instances

This patch replaces the remaining instances, so with a hacked up base/util, and a few build config changes mailnews/mime actually builds and links.
Attachment #369520 - Flags: superreview?(neil)
Attachment #369520 - Flags: review?(neil)

Updated

9 years ago
Attachment #369520 - Flags: superreview?(neil)
Attachment #369520 - Flags: superreview+
Attachment #369520 - Flags: review?(neil)
Attachment #369520 - Flags: review+
(Assignee)

Updated

9 years ago
Attachment #369520 - Attachment description: Remaining instances → [checked in] Remaining instances
(Assignee)

Comment 14

9 years ago
Comment on attachment 369520 [details] [diff] [review]
[checked in] Remaining instances

Checked in: http://hg.mozilla.org/comm-central/rev/88b32fe92103
(Assignee)

Comment 15

9 years ago
We'll call this fixed, any regressions can be dealt with in new bugs.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Summary: Move mailnews/mime to frozen linkage → Make mailnews/mime compile and link capable with frozen linkage
You need to log in before you can comment on or make changes to this bug.