Last Comment Bug 864256 - Build failure in nsMsgStatusFeedback.cpp: "error: 'class nsIXULBrowserWindow' has no member named 'SetJSDefaultStatus'"
: Build failure in nsMsgStatusFeedback.cpp: "error: 'class nsIXULBrowserWindow'...
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- blocker (vote)
: Thunderbird 23.0
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 862917
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-22 01:36 PDT by Frank Wein [:mcsmurf]
Modified: 2013-04-27 10:45 PDT (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Possible patch (3.77 KB, patch)
2013-04-23 00:37 PDT, neil@parkwaycc.co.uk
philip.chee: feedback+
Details | Diff | Splinter Review
Proposed patch (4.91 KB, patch)
2013-04-25 16:30 PDT, neil@parkwaycc.co.uk
standard8: review+
bugzilla: feedback+
Details | Diff | Splinter Review

Description Frank Wein [:mcsmurf] 2013-04-22 01:36:24 PDT
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 2013-04-22 04:10:30 PDT
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) 2013-04-22 05:06:34 PDT
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 [:bz] (TPAC) 2013-04-22 06:34:20 PDT
Or we could restore nsIXULBrowserWindow::SetJSDefaultStatus, if it actually has a consumer and an implementation in Thunderbird...
Comment 4 neil@parkwaycc.co.uk 2013-04-22 16:40:35 PDT
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 neil@parkwaycc.co.uk 2013-04-23 00:37:11 PDT
Created attachment 740689 [details] [diff] [review]
Possible patch
Comment 6 neil@parkwaycc.co.uk 2013-04-23 00:37:59 PDT
Comment on attachment 740689 [details] [diff] [review]
Possible patch

(Autocomplete fail...)
Comment 7 Philip Chee 2013-04-23 05:43:59 PDT
Comment on attachment 740689 [details] [diff] [review]
Possible patch

WORKSFORME. f=Ratty
Comment 8 Hartmut Figge 2013-04-23 11:10:50 PDT
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 2013-04-23 11:33:03 PDT
As a check yet another build with Proposed patch. The progress bar in MailNews again doesn't stop to spin.
Comment 10 Hartmut Figge 2013-04-23 11:41:21 PDT
(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 neil@parkwaycc.co.uk 2013-04-23 16:15:59 PDT
(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 2013-04-23 16:44:45 PDT
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 2013-04-25 03:37:15 PDT
> 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 neil@parkwaycc.co.uk 2013-04-25 16:30:41 PDT
Created attachment 742083 [details] [diff] [review]
Proposed patch

Seems reasonable.
Comment 15 Hartmut Figge 2013-04-25 18:03:50 PDT
(In reply to neil@parkwaycc.co.uk from comment #14)
> Created attachment 742083 [details] [diff] [review]

WFM on SM Linux x86_64
Comment 16 Frank Wein [:mcsmurf] 2013-04-27 03:03:59 PDT
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] 2013-04-27 03:38:52 PDT
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) 2013-04-27 10:15:30 PDT
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 (⌚ UTC+1/+2) 2013-04-27 10:45:50 PDT
https://hg.mozilla.org/comm-central/rev/dabe938d0e09

Note You need to log in before you can comment on or make changes to this bug.