Closed
Bug 315346
Opened 19 years ago
Closed 19 years ago
nsISidebar should be moved to dom/
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
References
Details
(Keywords: fixed1.8.1)
Attachments
(3 files)
33.09 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
18.82 KB,
patch
|
benjamin
:
review+
roc
:
superreview+
jst
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
28.34 KB,
patch
|
roc
:
review+
jst
:
superreview+
jst
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
The nsISidebar interface defines a public-facing DOM interface (window.sidebar). It doesn't make sense to have it forked in browser/ and xpfe/ since if they diverge, we'd break compatibility with Web sites. Furthermore, XULrunner embedders might want to implement this interface. Ephiphany does, for example. Rather than copy it for every embedder, the right thing to do is move it somewhere that's part of XULRunner. I claim dom/ is the right place.
Assignee | ||
Comment 1•19 years ago
|
||
This patch does that.
Attachment #202056 -
Flags: superreview?(jst)
Attachment #202056 -
Flags: review?(jst)
Comment 2•19 years ago
|
||
Comment on attachment 202056 [details] [diff] [review] simple fix r+sr=jst, but the nsIPermission changes are unrealted to the nsISidebar changes here, right? If not, you should get darin to approve that before landing that change...
Updated•19 years ago
|
Attachment #202056 -
Flags: superreview?(jst)
Attachment #202056 -
Flags: superreview+
Attachment #202056 -
Flags: review?(jst)
Attachment #202056 -
Flags: review+
Comment 3•19 years ago
|
||
Please get some cvs admin to copy the file in the repository to preserve cvs history :) jst: the nsIPermission thing is covered in a separate bug.
Assignee | ||
Comment 4•19 years ago
|
||
checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 5•19 years ago
|
||
shouldn't nsISidebar be removed from the old places?
Assignee | ||
Comment 6•19 years ago
|
||
Yeah
Comment 7•19 years ago
|
||
This caused bug 320370 and bug 321459 since the installer manifests weren't updated correctly. For some, dom_sidebar.xpt was added but sidebar.xpt wasn't removed (shouldn't it be?), and for others dom_sidebar.xpt wasn't added at all. Seems as though the old files still need to be removed, too. Is http://gavinsharp.com/tmp/tmp.diff correct?
Blocks: 321459
Comment 8•19 years ago
|
||
Attachment #207174 -
Flags: superreview?(roc)
Attachment #207174 -
Flags: review?(benjamin)
Updated•19 years ago
|
Attachment #207174 -
Flags: review?(benjamin) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #207174 -
Flags: superreview?(roc) → superreview+
Comment 9•18 years ago
|
||
Landed attachment 207174 [details] [diff] [review] on the trunk. mozilla/allmakefiles.sh; new revision: 1.599; mozilla/browser/components/sidebar/public/Makefile.in; delete; mozilla/browser/components/sidebar/public/nsISidebar.idl; delete; mozilla/xpfe/components/sidebar/public/.cvsignore; delete; mozilla/xpfe/components/sidebar/public/Makefile.in; delete; mozilla/xpfe/components/sidebar/public/nsISidebar.idl; delete;
Comment 10•18 years ago
|
||
Comment on attachment 207174 [details] [diff] [review] cvs remove browser/components/sidebar/public and xpfe/components/sidebar/public Shouldn't this be on the branch too, given that mozilla/browser/ changes are supposed to be synced between trunk and branch?
Attachment #207174 -
Flags: approval-branch-1.8.1?(peterv)
Updated•18 years ago
|
Attachment #207174 -
Flags: approval-branch-1.8.1?(peterv) → approval-branch-1.8.1?(jst)
Updated•18 years ago
|
Attachment #207174 -
Flags: approval-branch-1.8.1?(jst) → approval-branch-1.8.1+
Comment 11•18 years ago
|
||
Hmm, looks like the previous patches don't apply cleanly to the branch. Here's a patch containing the nsISidebar changes for the branch.
Attachment #220467 -
Flags: superreview?(jst)
Attachment #220467 -
Flags: review?(roc)
Attachment #220467 -
Flags: approval-branch-1.8.1?(jst)
Assignee | ||
Updated•18 years ago
|
Attachment #220467 -
Flags: review?(roc) → review+
Comment 12•18 years ago
|
||
Comment on attachment 220467 [details] [diff] [review] patch w/nsISidebar changes for branch sr+a=jst
Attachment #220467 -
Flags: superreview?(jst)
Attachment #220467 -
Flags: superreview+
Attachment #220467 -
Flags: approval-branch-1.8.1?(jst)
Attachment #220467 -
Flags: approval-branch-1.8.1+
Comment 13•18 years ago
|
||
Landed attachment 220467 [details] [diff] [review] on the branch.
Updated•18 years ago
|
Keywords: fixed1.8.1
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•