338070 - Revert the nsISidebar changes made in bug 334471

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

See bug 334471 comment 22 and following.

Comment 1

[Boris Zbarsky [:bzbarsky]]

In fact, it'd be great if we could get this done by 2.0a3....

Comment 2

[Asaf Romano (gone)]

taking.

Comment 3

[Asaf Romano (gone)]

Ugh, more fun at http://lxr.mozilla.org/seamonkey/source/dom/public/idl/sidebar/nsISidebar.idl#61

Comment 4

[Asaf Romano (gone)]

Attachment: branch patch

Comment 5

[Asaf Romano (gone)]

Comment on attachment 222464 [details] [diff] [review]
branch patch

will add a semicolon on checkin...

Comment 6

[Asaf Romano (gone)]

Attachment: trunk patch

Comment 7

[[e-mail]]

I don't know about anyone else but I'd quite like nsISidebar_MOZILLA_1_8_BRANCH renamed to nsISidebar2 and the patch checked in on the trunk too.

Comment 8

[Asaf Romano (gone)]

(In reply to comment #7)
> I don't know about anyone else but I'd quite like nsISidebar_MOZILLA_1_8_BRANCH
> renamed to nsISidebar2 and the patch checked in on the trunk too.
> 

FYI https://bugzilla.mozilla.org/show_bug.cgi?id=315346

If we're going to expose a different API, why should this live in dom/public?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't understand the question ... it's a DOM API, so why not dom/public?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 222469 [details] [diff] [review]
trunk patch

No need to change the CID.  sr=bzbarsky with that change removed.

Comment 11

[Boris Zbarsky [:bzbarsky]]

No preferences here on whether we use nsISidebar2 or just change nsISidebar on trunk.  If we do create an nsISidebar2, it needs to go in a separate IDL file, imo.

Comment 12

[Asaf Romano (gone)]

Boris, i changed the cid constant since it does not reflect reality, see http://lxr.mozilla.org/seamonkey/source/browser/components/sidebar/src/nsSidebar.js#60

Comment 13

[Asaf Romano (gone)]

(In reply to comment #9)
> I don't understand the question ... it's a DOM API, so why not dom/public?

The api is different between apps (seamonkey does not support the new method, Camino does not support window.sidebar at all IIRC), see comment 7.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #13)
> The api is different between apps (seamonkey does not support the new method,
> Camino does not support window.sidebar at all IIRC), see comment 7.

That should be fixed (Camino can just ignore the methods). Gecko-based browsers should export the same set of DOM interfaces to Web content.

Comment 15

[[e-mail]]

What's the utility of having SeaMonkey expose a new method it can't implement?
i.e. what's wrong with exposing the same DOM interface as 1.7 exposed?

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Consistency of DOM interfaces.

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

but since web content needs to be prepared for this function not existing anyway (other and older browsers don't have it), wouldn't it be better if web apps could detect that the function won't work?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's the same as any other Web API: we can't create consistency in other browsers or older versions of Gecko, that's beyond our control. But we should still make our Gecko releases as consistent as possible.

Comment 19

[[e-mail]]

Comment on attachment 222469 [details] [diff] [review]
trunk patch

<roc> if a Gecko-based browser doesn't HAVE a sidebar, or microsummaries, or whatever, the APIs should just do nothing

Out of interest, should trunk nsISidebar's IID be the same as that of branch nsISidebar_MOZILLA_1_8_BRANCH?

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm. I don't know.

Comment 21

[Asaf Romano (gone)]

1.8 branch patch:
mozilla/dom/public/idl/sidebar/nsISidebar.idl 1.3.10.4
mozilla/browser/components/sidebar/src/nsSidebar.js 1.10.8.4

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Per bz, the IDL change without IID rev definitely needs to block 1.9.

Comment 23

[Mike Schroepfer]

Ping - did we forget this on trunk?

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #19)
> Out of interest, should trunk nsISidebar's IID be the same as that of branch
> nsISidebar_MOZILLA_1_8_BRANCH?

Let's make it different, just to be on the safe side.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Trivial UUID change

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 269115 [details] [diff] [review]
fix

r+sr=dbaron.  Sorry for the delay.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked that in.