165653 - mozilla/dom depends on mozilla/xpfe/components/sidebar (nsISideBar)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

mozilla/dom depends on mozilla/xpfe/components/sidebar  (nsISideBar)

talking to alecf, embeddors shouldn't need pull or build
mozilla/xpfe/components/sidebar.

Comment 1

[Alec Flett]

mozilla 1.1alpha is more or less done, so I'm moving non-critical mozilla1.2beta
bugs out to the next milestone to make room for the mozilla1.1alpha bugs that
didn't make it.

Comment 2

[Alec Flett]

moving non-critical 1.3alpha bugs to 1.4alpha

Comment 3

[Alec Flett]

mass moving lower risk 1.4alpha stuff to 1.4beta

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking, I've got the fix.

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch

So this patch makes the prompter get the active window instead of the calling
window (since there really is no way for us to know what the calling window was
otherwise), but that should not be that big of a deal, really.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 130260 [details] [diff] [review]
Patch

jst should definitely review this for the dom changes, but I'm not sure who
else to ping about this.  bryner, perhaps?

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 130260 [details] [diff] [review]
Patch

Looks good to me, assuming JAVASCRIPT_GLOBAL_PROPERTY_CATEGORY works the way I
think it does.	It's unfortunate that we can't get the calling window though...
I wonder about maybe adding an nsIObserver notification after an instance is
created from GLOBAL_PROPERTY_CATEGORY, which would pass in the DOM window. 
Might be a performance loss though, and as you say, it's not a huge deal.

r=bryner.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 130260 [details] [diff] [review]
Patch

sr=jst

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Fix checked in.

Comment 10

[Steffen Wilberg]

This probably caused the regression in bug 219534.

Comment 11

[Erich 'Ricky' Iseli]

Not probably, it did cause the regression. I have not enough knowledge about
this bug, but I really cannot understand why this was removed. If the search
engines should not be added to window.sidebar, then I'm fine, since the search
box actually isn't the sidebar. But then we need a new way to add the search
engines.

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

The regression is firebird only.  Note that I did not remove sidebar at all.  I
just "moved" it.  It used to be that gecko depended on it, meaning you could not
have a mozilla without it.  It got moved to seamonkey.  Firebird just needs to
have the last 2 files of the patch ported over.  I can't do that since I don't
have access to firebird.

Comment 13

[Asa Dotzler [:asa]]

Chris, when you said "I can't do that since I don't have access to firebird,"
did you really mean "Screw Firebird, I'm not interested in fixing my regression
there" because "Firebird just needs to have the last 2 files of the patch ported
over. I can't do that since I don't have access to firebird" isn't quite the truth. 
As far as I can tell, there's nothing about mozilla.org's system that prevents
you from accessing the Firebird code, so you certainly could port those files
over and offer a patch to the Firebird developers for check-in. 

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

No, I meant that the patch is here and there really is not much more I can do.

If someone grants me access to the firebird partition, I'd be more than happy to
fix the regression myself.  Or if someone who has access to it grabs the patch
and applies it from browser/ with |patch -p1| ignoring the dom stuff (just keep
tapping enter) that would work too.  I'm not all that inclined to grab a
firebird tree to produce a pretty much identical patch, especially since my
build environment is hosed.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Note that the firebird partition is now open.

Asa, chill out.

Comment 16

[Christopher Aillon (sabbatical, not receiving bugmail)]

Oh, yeah the firebird thing was fixed a long time ago, and pch had a maintenance
patch to it recently since there was one additional caller of nsISidebar.window
that I didn't notice.  viewing sidebar worked but adding them didn't.  That
should also be fixed now.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Hrm... so this patch makes window.sidebar always point to our sidebar... this
breaks sites that have a frame named "sidebar".  See bug 222191.  That's not
cool.  :(