465073 - duplicate id in navigator.xul: reporterItemsBroadcaster

Status: VERIFIED FIXED

(Other Applications Graveyard :: Reporter, defect)

Description

[Robert Kaiser]

We have a chrome mochitest error since bug 428216 has been checked in, reporting reporterItemsBroadcaster as a duplicate ID in navigator.xul. This is caused by http://mxr.mozilla.org/comm-central/source/mozilla/extensions/reporter/resources/content/reporter/reporterOverlay.xul suddenly succeeding to overlay both its broadcaster definitions as http://hg.mozilla.org/comm-central/annotate/f46a87cfb094/suite/browser/navigator.xul#l115 was added by bug 428216.

We need to solve that in some way, which could even mean to remove the SeaMonkey-specific parts of the reporter overlay in mozilla-central.

Comment 1

[Stefan [:stefanh]]

I guess there are 2 ways:

1) Remove the broadcaster from navigator.xul
2) Accept some changes in the Help menu. In utilityOverlay.xul, we can use "menu_HelpPopup" instead of "helpPopup" and "releaseNotes" instead of "releaseURL".

Comment 2

[Stefan [:stefanh]]

(I'm going to try 2) and see how it looks)

Comment 3

[Philip Chee]

Urm?? Why not just remove <broadcasterset id="navBroadcasters"> from reporterOverlay.xul?

Comment 4

[Stefan [:stefanh]]

(In reply to comment #3)
> Urm?? Why not just remove <broadcasterset id="navBroadcasters"> from
> reporterOverlay.xul?

Yeah, that might be better. I think we'll need a separate bug filed for that. Would you mind filing it?

Comment 5

[Robert Kaiser]

Why do we need a separate bug for resolving this bug? If someone comes up with a patch and we decide it's actually reporter that needs to be fixed (I actually tend to agree on that), then Bugzilla has this cool feature of being able to just move the bug to that product/component.
That's why I filed that bug on the symptom, not the cause, as whatever fix fixes the symptom of the failing test fixes this bug.

Comment 6

[Stefan [:stefanh]]

I guess my mind was still at 2) in comment #1 (and that would have been this bug + another bug imo)

Comment 7

[Philip Chee]

Attachment: Remove "navBroadcasters" from overlay

Now that we have a "mainBroadcasterSet" we should simply remove "navBroadcasters".

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 348535 [details] [diff] [review]
Remove "navBroadcasters" from overlay

>diff --git a/extensions/reporter/resources/content/reporter/reporterOverlay.xul b/extensions/reporter/resources/content/reporter/reporterOverlay.xul

>+  <!-- All Applications -->

s/All Applications/SeaMonkey and Firefox/ ?

Comment 9

[Philip Chee]

> s/All Applications/SeaMonkey and Firefox/ ?
I considered it but it looked too verbose. Perhaps we could remove this comment entirely as it's probably superfluous.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Too verbose? It's three words! :) I think it makes sense to be precise and explicitly state which apps the overlay was meant to support.

Comment 11

[Philip Chee]

Attachment: Fix comment nit, c/f r+

> Too verbose? It's three words! :) I think it makes sense to be precise and
> explicitly state which apps the overlay was meant to support.

Fixed comment to say <!-- Firefox and SeaMonkey -->
Carrying forward r+

I think that this is NPOTFB, nevertheless do I need a+ for landing in mozilla-central ATM?

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> I think that this is NPOTFB, nevertheless do I need a+ for landing in
> mozilla-central ATM?

Approval is not needed for NPOTDB, but checkins are currently metered:
https://wiki.mozilla.org/1.9.1_beta_2_checkins

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Go ahead and check it in whenever.

Comment 14

[Robert Kaiser]

Pushed as http://hg.mozilla.org/mozilla-central/rev/813830032772 with sheriff permission :)

Comment 15

[Serge Gautherie (:sgautherie)]

Tinderboxes went green after 2008/11/18 10:18:06.

V.Fixed