516679 - Missing message header view after bug 515729 checkin

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Robert Kaiser]

bug 515729 completely broke display of SeaMonkey's header view due to changing the shared SMIME overlay.

I get the following in error console:

Error: gSMIMEContainer is null
Source File: chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js
Line: 181

I didn't understand why this would make us show no header view whatsoever, Neil said this on IRC:
<NeilAway> KaiRo: it's hidden by default
<NeilAway> KaiRo: so since the error is thrown, nobody unhides it


I confirmed that a local backout of http://hg.mozilla.org/comm-central/rev/77d79dc4ef45 fixes the problem.

Comment 1

[Philip Chee]

msgHdrViewSMIMEOverlay.xul should be forked. Thunderbird already forks some of the smime code.

Comment 2

[Robert Kaiser]

IMHO, this shouldn't even be an overlay, we don't need to support shipping without crypto any more.

Comment 3

[Mark Banner (:standard8)]

Attachment: Proposed fix

Proposed fix. Whilst I don't like forking stuff unnecessary, I think it is clear here the smime isn't really an extension and the message header UIs are now diversifying.

This patch therefore forks msgHdrViewSMIMEOverlay.xul and reverts the change made in bug 515729.

With respect to Robert's comment about it not needing to be an extension and should be merged into the main file - I agree. However, I think in the run up to releases isn't the time to look at it and we should file follow up bugs to look at if/how/what to merge post our major releases.

Comment 4

[Robert Kaiser]

Mark, I fully agree that it might not be the right time to do this right now, I was more talking in general there. Probably we should file a followup on that.

Comment 5

[Simon Paquet [:sipaq]]

Was the followup bug already filed?

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> Was the followup bug already filed?

No, I'll file them before the bug is closed. That way if someone decides to reject my proposal I don't waste my time.

Comment 7

[[e-mail]]

[Sadly due to a design flaw in hg we can't easily fork the old version of the file. Fortunately we have no useful hg history to lose by this backout.]

Comment 8

[Karsten Düsterloh]

Comment on attachment 400987 [details] [diff] [review]
Proposed fix

>+   content/messenger-smime/msgHdrViewSMIMEOverlay.xul               (content/msgHdrViewSMIMEOverlay.xul)

You need to mark this line with a leading + so that the file overwrites the one from /mailnews.

I built both SM and TB with this and both have smimeBox at the wanted place in the DOM.


r=me with that addition.

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> (From update of attachment 400987 [details] [diff] [review])
> >+   content/messenger-smime/msgHdrViewSMIMEOverlay.xul               (content/msgHdrViewSMIMEOverlay.xul)
> 
> You need to mark this line with a leading + so that the file overwrites the one
> from /mailnews.

No I don't - the mailnews/ jar.mn file is ifdef MOZ_SUITE:

http://mxr.mozilla.org/comm-central/source/mailnews/extensions/smime/jar.mn

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 400987 [details] [diff] [review]
Proposed fix

sr=dmose

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 400987 [details] [diff] [review]
Proposed fix

Approving my own patch as this is essentially just a bustage fix and I'm trying to get it into today's nightlies.

Comment 12

[Mark Banner (:standard8)]

Raised bug 517175 on the SMIME merge.

Checked in:

http://hg.mozilla.org/comm-central/rev/473fdca7dbd3

This should be in for all of today's nightlies.

Comment 14

[Wade Reese]

Hi Philip,

Is this fixed or is it in process?  If not fixed, do you have an idea as to when or is that hard to predict?  Thanks.

Wade

Comment 15

[Wade Reese]

Seems to be fixed with the build from today.  Cheers!