Closed Bug 469606 Opened 16 years ago Closed 15 years ago

|nsEudoraEditor.cpp(134) : error C2511: 'nsresult nsEudoraEditor::PreDestroy(void)' : overloaded member function not found in 'nsEudoraEditor'|, with m-c trunk

Categories

(MailNews Core :: Import, defect)

defect
Not set
blocker

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: sgautherie, Assigned: sgautherie)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20081127 SeaMonkey/2.0a2pre] (home, debug default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/28a7fa014b03
+http://hg.mozilla.org/comm-central/rev/3c516d1c1248 + bug 519592 patch)

Last build I made, which went fine.

***

Current tree, which I tried 3 times...:
(http://hg.mozilla.org/mozilla-central/rev/e8d4c06419db
+http://hg.mozilla.org/comm-central/rev/d88275bc5d35)

{
nsEudoraEditor.cpp
.../mailnews/import/eudora/src/nsEudoraEditor.cpp(134) : error C2511: 'nsresult nsEudoraEditor::PreDestroy(void)' : overloaded member function not found in 'nsEudoraEditor'
        ...\mailnews\import\eudora\src\nsEudoraEditor.h(48) : see declaration of 'nsEudoraEditor'
}

NB: That's unexpected, as the Windows SM tinderboxes never reported this :-/
Odder, none of these files have been touched in the timeframe...
(In reply to comment #1)

Actually, it did:
http://hg.mozilla.org/mozilla-central/rev/2cbcaca3fcdb

Only,
bug 468698 prevents to see it through MXR
and "You are not authorized to access bug #459613."
;-/

Thanks to |hg grep -r ...:... --all PreDestroy| :-)
Flags: blocking1.9.1?
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081215 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/2c2d64b8566c
 +http://hg.mozilla.org/comm-central/rev/d88275bc5d35 + bug 469606 patch)

Trivial fix.

***

(In reply to comment #0)
> 
> NB: That's unexpected, as the Windows SM tinderboxes never reported this :-/

Ah ... I'm on m-c trunk whereas the SM boxes are on m-c-1.9.1 branch ! :->
(Which is the bug about adding (at least 1) trunk SeaMonkey/Thunderbird boxes ?)
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #352979 - Flags: superreview?(bienvenu)
Attachment #352979 - Flags: review?(bienvenu)
Flags: blocking1.9.1? → in-testsuite-
OS: Windows 2000 → All
Hardware: PC → All
Summary: |nsEudoraEditor.cpp(134) : error C2511: 'nsresult nsEudoraEditor::PreDestroy(void)' : overloaded member function not found in 'nsEudoraEditor'|, on my Windows 2000 → |nsEudoraEditor.cpp(134) : error C2511: 'nsresult nsEudoraEditor::PreDestroy(void)' : overloaded member function not found in 'nsEudoraEditor'|, with m-c trunk
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
Attached patch (Av1a) Bug 459613 followup (obsolete) — Splinter Review
Av1, but supporting both trunk and branch :->

It looks like to be the first patch to need this |#ifdef| ... unless bug 459613 would land (now) on branch too...
Attachment #352979 - Attachment is obsolete: true
Attachment #352985 - Flags: superreview?(bienvenu)
Attachment #352985 - Flags: review?(roc)
Attachment #352979 - Flags: superreview?(bienvenu)
Attachment #352979 - Flags: review?(bienvenu)
Serge, please don't bother with this patch at the moment. We're not building mozilla-central via tinderboxes, and the development focus is 1.9.1. Additionally I've been told that bug 459613 is due to land on the branch anyway. So we may as well just land the relevant change to both when it lands on 1.9.1, and skip the need for an intermediate changeset in this case.
Since the only method that nsMsgSend calls on its nsIEditor is OutputToString (to get the body in HTML), maybe we should add a method to encapsulate that on nsIEditorMailSupport so that nsEudoraEditor doesn't have to pretend to implement the whole of nsIEditor?
Comment on attachment 352985 [details] [diff] [review]
(Av1a) Bug 459613 followup

looks ok, thx, modulo Neil's and Standard8's comments.
Attachment #352985 - Flags: superreview?(bienvenu) → superreview+
(In reply to comment #5)
> Serge, please don't bother with this patch at the moment. We're not building
> mozilla-central via tinderboxes, and the development focus is 1.9.1.

This (MacOSX and Windows) bug can now be monitored on new "m-c"
http://tinderbox.mozilla.org/showbuilds.cgi?tree=Thunderbird
(In reply to comment #8)
> (In reply to comment #5)
> > Serge, please don't bother with this patch at the moment. We're not building
> > mozilla-central via tinderboxes, and the development focus is 1.9.1.
> 
> This (MacOSX and Windows) bug can now be monitored on new "m-c"
> http://tinderbox.mozilla.org/showbuilds.cgi?tree=Thunderbird

Which as we have already explained are low priority, test boxes and you shouldn't need to monitor them day to day. We know the errors, and even with that I would still prefer that we only fix this bug once bug 459613 lands so that we only need one changeset.
Depends on: 459613
not a blocker to current development paths.
Severity: blocker → critical
(In reply to comment #5)
> Serge, please don't bother with this patch at the moment. We're not building

I bothered because I needed it to compile my local build, at least.

> mozilla-central via tinderboxes, and the development focus is 1.9.1.

I know, though the former is already "fixed".

> Additionally I've been told that bug 459613 is due to land on the branch
> anyway.

Good, then we can use Av1 patch :-)

(In reply to comment #9)
> Which as we have already explained are low priority, test boxes and you

(Did I write anything different ?)

> shouldn't need to monitor them day to day. We know the errors, and even with

I was not: it just happened I've been building SeaMonkey (and running tests), which I had not done in some time.

> that I would still prefer that we only fix this bug once bug 459613 lands so
> that we only need one changeset.

(Did I write anything different ?)
(In reply to comment #11)
> (In reply to comment #5)
> > Serge, please don't bother with this patch at the moment. We're not building
> 
> I bothered because I needed it to compile my local build, at least.

Please accept my apologies I should have phrased that differently.

> > Additionally I've been told that bug 459613 is due to land on the branch
> > anyway.
> 
> Good, then we can use Av1 patch :-)

Yes, somehow I hadn't noticed that one.

> (In reply to comment #9)
> > Which as we have already explained are low priority, test boxes and you
> 
> (Did I write anything different ?)
> 
> > shouldn't need to monitor them day to day. We know the errors, and even with
> 
> I was not: it just happened I've been building SeaMonkey (and running tests),
> which I had not done in some time.
> 
> > that I would still prefer that we only fix this bug once bug 459613 lands so
> > that we only need one changeset.
> 
> (Did I write anything different ?)

Ok, what got me here was the comment about the builds on the Thunderbird tree. That really didn't seem necessary as its obvious that they were pointing to this bug, and anyone on this bug would be able to correlate the two should they have seen the Thunderbird tree. So therefore it appeared the only reason you were commenting there was to push the issue.


A little aside: I've just been chatting with Dan on irc, the general thoughts now are that we need to keep c-c and m-c in a reasonable state, i.e. not burning and working to some extent. We need to track regressions, though we may not fix them straight away.

With respect to the current breakage, we have decided that we'll run with it until either bug 459613 lands on branch, or dmose needs to land a mozilla-central patch before he lands it on mozilla-1.9.1. Once we get past that stage we'll be trying to keep from bustages as a minimum, and tracking performance number regressions (though I suspect most of these will be in m-c if not showing on c-c plus m-191).
Attachment #352985 - Flags: review?(roc) → review+
Comment on attachment 352985 [details] [diff] [review]
(Av1a) Bug 459613 followup

Pre-emptively setting r+. If dmose wants to land his mozilla-central patch before we need this, then r=me, else...
Attachment #352979 - Attachment is obsolete: false
Attachment #352979 - Flags: superreview+
Attachment #352979 - Flags: review+
Comment on attachment 352979 [details] [diff] [review]
(Av1) Bug 459613 followup
[Checkin: Comment 16]

...r/sr=me for this patch if the bug that affects us lands on 1.9.1 first.
Whiteboard: [see comments 12-14 for which patch to land when]
(In reply to comment #12)

> Please accept my apologies I should have phrased that differently.

Accepted.

> Ok, what got me here was the comment about the builds on the Thunderbird tree.
> That really didn't seem necessary as its obvious that they were pointing to

To be obvious, the log needs to be opened, the bug known, etc.
I can't see what's wrong in adding 1 comment, with the bug number.

Also that's when I found out this bug affects MacOSX too and not Windows only.

> this bug, and anyone on this bug would be able to correlate the two should they
> have seen the Thunderbird tree. So therefore it appeared the only reason you

One of the points of comment 8 was to move from something I reported on (my) local build to something which can be seen on official boxes, etc.
I can't see what's wrong with that either.

> were commenting there was to push the issue.

Actually no: it happened the other way round, first I filed the bug, then gozer set up the new TB boxes (which I thank him/you for !) ... I was merely connecting the two of them.

> A little aside: I've just been chatting with Dan on irc, the general thoughts
> now are that we need to keep c-c and m-c in a reasonable state, i.e. not
> burning and working to some extent. We need to track regressions, though we may
> not fix them straight away.

This is just how I (wanted to) understood it from day one :->
And why I filed bug 470184, even before reading your comment 12 here.
http://hg.mozilla.org/comm-central/rev/ccf2f94dbcca
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #352985 - Attachment is obsolete: true
Attachment #352979 - Attachment description: (Av1) Bug 459613 followup → (Av1) Bug 459613 followup [Checkin: Comment 16]
V.Fixed, as the 2 TB/m-c boxes don't have this error anymore.
Severity: critical → blocker
Status: RESOLVED → VERIFIED
Whiteboard: [see comments 12-14 for which patch to land when]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
You need to log in before you can comment on or make changes to this bug.