864256 - Build failure in nsMsgStatusFeedback.cpp: "error: 'class nsIXULBrowserWindow' has no member named 'SetJSDefaultStatus'"

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Frank Wein [:mcsmurf]]

Since Bug 862917 landed on mozilla-central, the Thunderbird and SeaMonkey builds fail with this error:
../../../../mailnews/base/src/nsMsgStatusFeedback.cpp: In member function 'virtual nsresult nsMsgStatusFeedback::SetStatusString(const nsAString_internal&)':
../../../../mailnews/base/src/nsMsgStatusFeedback.cpp:202:23: error: 'class nsIXULBrowserWindow' has no member named 'SetJSDefaultStatus'
make[8]: *** [nsMsgStatusFeedback.o] Error 1

I'm not really sure how to fix this.

Comment 1

[Philip Chee]

q.v.
  Bug 862917 - Remove window.defaultStatus.
  Bug 842017 - Remove broken support for displaying window.status / window.defaultStatus.
  Bug 862540 - window.status can no longer be set to anything, ever.
  Bug 863339 - remove unneeded code that supports setting window.status.

> I'm not really sure how to fix this.
Dunno. If Bug 863339 lands then I suppose we could replace "SetJSDefaultStatus" with "SetOverLink"

Comment 2

[Mark Banner (:standard8)]

The default status is just a fallback for if we're not displaying any status text. We should be able to write some code to take account for that and just replace what was there I believe.

I'm not going to get time for this over the next week, though you might be able to ping me on irc if you want ideas. This code has felt a bit messy in the past, so a tidy up wouldn't hurt.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Or we could restore nsIXULBrowserWindow::SetJSDefaultStatus, if it actually has a consumer and an implementation in Thunderbird...

Comment 4

[[e-mail]]

I think we can work around this by renaming the JS method name to setStatusString, which will allow us to call it on the nsIMsgStatusFeedback interface. (But my build is busted right now for other reasons which is why I don't have a patch ready yet.)

Comment 5

[[e-mail]]

Attachment: Possible patch

Comment 6

[[e-mail]]

Comment on attachment 740689 [details] [diff] [review]
Possible patch

(Autocomplete fail...)

Comment 7

[Philip Chee]

Comment on attachment 740689 [details] [diff] [review]
Possible patch

WORKSFORME. f=Ratty

Comment 8

[Hartmut Figge]

A build with Possible patch shows a non-stopping progress bar on MailNews. A subsequent build without this patch, instead with a backout of the patch of Bug 864256, does not show this unpleasant behavior.

SM Linux x86_64, Modern

Comment 9

[Hartmut Figge]

As a check yet another build with Proposed patch. The progress bar in MailNews again doesn't stop to spin.

Comment 10

[Hartmut Figge]

(In reply to Hartmut Figge from comment #9)
> As a check yet another build with Proposed patch. The progress bar in
> MailNews again doesn't stop to spin.

Probably due to
Error: ReferenceError: defaultStatus is not defined
Source File: chrome://messenger/content/mailWindow.js
Line: 388

Comment 11

[[e-mail]]

(In reply to Hartmut Figge from comment #10)
> (In reply to Hartmut Figge from comment #9)
> > As a check yet another build with Proposed patch. The progress bar in
> > MailNews again doesn't stop to spin.
> 
> Probably due to
> Error: ReferenceError: defaultStatus is not defined
> Source File: chrome://messenger/content/mailWindow.js
> Line: 388

Ah, that must have been accidentally using window.defaultStatus which was also removed as part of bug 862917...

Comment 12

[Hartmut Figge]

That reminds me of the C&P error in comment 8. Should have been 'backout of the patch of Bug 862917'. Sigh.

Comment 13

[Philip Chee]

> Ah, that must have been accidentally using window.defaultStatus which was also removed
> as part of bug 862917...
I wonder if Bienvenu meant to use "myDefaultStatus" instead in Bug 181627

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/base/resources/content/mailWindow.js&rev=1.100.2.6&mark=426#418

Comment 14

[[e-mail]]

Attachment: Proposed patch

Seems reasonable.

Comment 15

[Hartmut Figge]

(In reply to [e-mail] from comment #14)
> Created attachment 742083 [details] [diff] [review]

WFM on SM Linux x86_64

Comment 16

[Frank Wein [:mcsmurf]]

Comment on attachment 742083 [details] [diff] [review]
Proposed patch

Tested the patch locally, status bar updates seem to work fine, progress bar also works fine. No errors are visible in JS Console.

Comment 17

[Joe Sabash [:JoeS1]]

TB builds OK as well:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Thunderbird/23.0a1 ID:20130426174337 CSet: 3267f9ecf52c

Comment 18

[Mark Banner (:standard8)]

Comment on attachment 742083 [details] [diff] [review]
Proposed patch

Review of attachment 742083 [details] [diff] [review]:
-----------------------------------------------------------------

I can't test this at the moment, but given others have, and the code looks fine, r=me.

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/comm-central/rev/dabe938d0e09