511242 - Include nsBrowserStatusFilter.cpp in Thunderbird's components list

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

As part of bug 511240 we'd like to use nsBrowserStatusFilter to help with content loading notifications.

Although Thunderbird currently builds in mozilla/xpfe/browser it doesn't build in xpfe/components/build which actually includes it in the components list.

I'm looking to get this fix into 1.9.1 so unfortunately the easiest way to do this without breaking anything or doing complicated things is with ifdefs. This is nasty, but if you let me do it, I'll look at removing nsBrowserInstance and moving nsBrowserStatusFilter to somewhere else for trunk builds ;-)

Dan: nsBrowserStatusFilter is a small component that will help us with updates to content loading status and save lots of state changes on the UI. Firefox and SeaMonkey both use it on their tabbrowsers.

Comment 1

[Mark Banner (:standard8)]

Marking for tb3needs at least for now - probably tb3wants is the right thing but I don't know what Dan is using yet. In any case, small patch that I'd like to track and get in.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 395167 [details] [diff] [review]
The fix

Dan is happy with this in principle and will get to it in the next day or so, hence requesting build config review.

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #0)
> Dan: nsBrowserStatusFilter is a small component that will help us with updates
> to content loading status and save lots of state changes on the UI. Firefox and
> SeaMonkey both use it on their tabbrowsers.

Just to extend a little, at the moment it is only used in tabbrowsers. Using the code I should also be able to copy the fav/site icon code from Firefox tabbrowser so that we have icons on pages as well.

Comment 4

[Mark Banner (:standard8)]

ted/dmose: ping for review, I'm looking to get this into 1.9.1.4 which means I need to get it on trunk soon.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 395167 [details] [diff] [review]
The fix

diff --git a/xpfe/components/build/Makefile.in b/xpfe/components/build/Makefile.in

-# Non-Mac Browser requirements
-ifneq ($(MOZ_BUILD_APP),macbrowser)
 SHARED_LIBRARY_LIBS += ../../browser/src/$(LIB_PREFIX)mozbrwsr_s.$(LIB_SUFFIX)
 LOCAL_INCLUDES += -I$(srcdir)/../../browser/src
-endif

I don't understand the point of this change.

I'm also not comfortable reviewing the changes to xpfe/components/build/nsModule.cpp, please get a peer for that.

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> (From update of attachment 395167 [details] [diff] [review])
> diff --git a/xpfe/components/build/Makefile.in
> b/xpfe/components/build/Makefile.in
> 
> -# Non-Mac Browser requirements
> -ifneq ($(MOZ_BUILD_APP),macbrowser)
>  SHARED_LIBRARY_LIBS += ../../browser/src/$(LIB_PREFIX)mozbrwsr_s.$(LIB_SUFFIX)
>  LOCAL_INCLUDES += -I$(srcdir)/../../browser/src
> -endif
> 
> I don't understand the point of this change.

I did that to simplify the ifdefs as much as I could and because we've basically removed all the Camino ifdefs apart from this one (http://mxr.mozilla.org/mozilla-central/search?string=ifneq.*macbrowser&regexp=on&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central)

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 395167 [details] [diff] [review]
The fix

Ok, thanks.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 395167 [details] [diff] [review]
The fix

Neil, can you take a quick look at the nsModule.cpp changes?

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 395167 [details] [diff] [review]
The fix

sr=dmose

Comment 10

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/mozilla-central/rev/3f2d457dc885

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 395167 [details] [diff] [review]
The fix

Requesting branch approvals, this is a build-config only patch that enables the nsBrowserStatusFilter component on Thunderbird so we can use it for filtering notifications in content tabs. Should be low risk as it is build-config only.

Comment 12

[Samuel Sidler (old account; do not CC)]

Comment on attachment 395167 [details] [diff] [review]
The fix

Approved for 1.9.1.4. a=ss

Comment 13

[Mark Banner (:standard8)]

Checked into 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8e398b757b87

Comment 14

[Mark Banner (:standard8)]

Checked into 1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9dfcea1e43f8

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1 by examining source.