Closed Bug 338070 Opened 15 years ago Closed 14 years ago

Revert the nsISidebar changes made in bug 334471

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta1

People

(Reporter: bzbarsky, Assigned: roc)

References

Details

(Keywords: fixed1.8.1)

Attachments

(3 files)

See bug 334471 comment 22 and following.
Flags: blocking1.9a1+
Flags: blocking1.8.1+
Summary: Revert the nsISidebar made in bug 334471 → Revert the nsISidebar changes made in bug 334471
In fact, it'd be great if we could get this done by 2.0a3....
taking.
Assignee: general → bugs.mano
Severity: normal → major
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1alpha3
Attached patch branch patchSplinter Review
Attachment #222464 - Flags: superreview?(bzbarsky)
Attachment #222464 - Flags: review?(mconnor)
Attachment #222464 - Flags: approval-branch-1.8.1?(bzbarsky)
Comment on attachment 222464 [details] [diff] [review]
branch patch

will add a semicolon on checkin...
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.
(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?
I don't understand the question ... it's a DOM API, so why not dom/public?
Attachment #222464 - Flags: superreview?(bzbarsky)
Attachment #222464 - Flags: superreview+
Attachment #222464 - Flags: approval-branch-1.8.1?(bzbarsky)
Attachment #222464 - Flags: approval-branch-1.8.1+
Comment on attachment 222469 [details] [diff] [review]
trunk patch

No need to change the CID.  sr=bzbarsky with that change removed.
Attachment #222469 - Flags: superreview?(bzbarsky) → superreview+
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.
(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.
(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.
Attachment #222464 - Flags: review?(mconnor) → review+
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?
Consistency of DOM interfaces.
Target Milestone: mozilla1.8.1alpha3 → mozilla1.8.1beta1
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?
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 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?
Attachment #222469 - Flags: review?(neil) → review-
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
Keywords: fixed1.8.1
Per bz, the IDL change without IID rev definitely needs to block 1.9.
Flags: blocking1.9+
Flags: blocking1.9a1+
Ping - did we forget this on trunk?
(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.
Attached patch fixSplinter Review
Trivial UUID change
Assignee: mano → roc
Attachment #269115 - Flags: superreview?(dbaron)
Attachment #269115 - Flags: review?(dbaron)
Comment on attachment 269115 [details] [diff] [review]
fix

r+sr=dbaron.  Sorry for the delay.
Attachment #269115 - Flags: superreview?(dbaron)
Attachment #269115 - Flags: superreview+
Attachment #269115 - Flags: review?(dbaron)
Attachment #269115 - Flags: review+
checked that in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.