Closed Bug 511242 Opened 11 years ago Closed 11 years ago

Include nsBrowserStatusFilter.cpp in Thunderbird's components list

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(status1.9.2 beta1-fixed, status1.9.1 .4-fixed)

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed
status1.9.1 --- .4-fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: verified1.9.1, Whiteboard: [tb3wanted][no l10n impact])

Attachments

(1 file)

Attached patch The fixSplinter Review
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.
Attachment #395167 - Flags: superreview?(dmose)
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.
Whiteboard: [tb3needs]
Whiteboard: [tb3needs] → [tb3needs][no l10n impact]
Whiteboard: [tb3needs][no l10n impact] → [tb3wants][no l10n impact]
Blocks: 511849
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.
Attachment #395167 - Flags: review?(ted.mielczarek)
Whiteboard: [tb3wants][no l10n impact] → [tb3wants][no l10n impact][needs review dmose,ted]
(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.
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 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.
Attachment #395167 - Flags: review?(ted.mielczarek) → review-
(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 on attachment 395167 [details] [diff] [review]
The fix

Ok, thanks.
Attachment #395167 - Flags: review- → review+
Comment on attachment 395167 [details] [diff] [review]
The fix

Neil, can you take a quick look at the nsModule.cpp changes?
Attachment #395167 - Flags: review?(neil)
Whiteboard: [tb3wants][no l10n impact][needs review dmose,ted] → [tb3wants][no l10n impact][needs review dmose,neil]
Attachment #395167 - Flags: review?(neil) → review+
Whiteboard: [tb3wants][no l10n impact][needs review dmose,neil] → [tb3wants][no l10n impact][needs review dmose]
Attachment #395167 - Flags: superreview?(dmose) → superreview+
Whiteboard: [tb3wants][no l10n impact][needs review dmose] → [tb3wants][no l10n impact]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [tb3wants][no l10n impact] → [tb3wants][no l10n impact][baking on trunk]
Target Milestone: --- → mozilla1.9.3a1
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.
Attachment #395167 - Flags: approval1.9.2?
Attachment #395167 - Flags: approval1.9.1.4?
Whiteboard: [tb3wants][no l10n impact][baking on trunk] → [tb3wants][no l10n impact][awaiting branch approvals]
Comment on attachment 395167 [details] [diff] [review]
The fix

Approved for 1.9.1.4. a=ss
Attachment #395167 - Flags: approval1.9.1.4? → approval1.9.1.4+
Checked into 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8e398b757b87
Whiteboard: [tb3wants][no l10n impact][awaiting branch approvals] → [tb3wanted][no l10n impact][awaiting 192 branch approvals]
Attachment #395167 - Flags: approval1.9.2? → approval1.9.2+
Checked into 1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9dfcea1e43f8
Whiteboard: [tb3wanted][no l10n impact][awaiting 192 branch approvals] → [tb3wanted][no l10n impact]
Verified for 1.9.1 by examining source.
Keywords: verified1.9.1
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.