Closed
Bug 129602
Opened 22 years ago
Closed 20 years ago
Embedders need to be able to reach the window root.
Categories
(Core Graveyard :: Embedding: APIs, defect)
Core Graveyard
Embedding: APIs
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jst, Assigned: mpgritti)
References
Details
(Keywords: embed, fixed1.7, topembed-, Whiteboard: fixed-aviary1.0)
Attachments
(2 files, 3 obsolete files)
2.82 KB,
patch
|
Details | Diff | Splinter Review | |
5.54 KB,
patch
|
chofmann
:
approval1.7+
|
Details | Diff | Splinter Review |
Embedders might want to reach the window root in embedding apps to register event listeners n' such. The most convenient place to do this is in nsIDOMWindow, but it must not be scriptable (since the window root might be a chrome window) so we'll need to mark this new attribute [noscript]. The new attribute should be of type nsIDOMEventTarget.
Updated•22 years ago
|
Looks like should go to 1.0 then.
Target Milestone: --- → mozilla1.0
Reporter | ||
Comment 2•22 years ago
|
||
Hyatt, this is what we want here, right?
Comment 3•22 years ago
|
||
topembed- since we don't know that this is a blocker for any embeddor. It does sound like something we may need in the future however
Comment 4•22 years ago
|
||
Here's an embedder who might find this useful! :-) I've been working on sidebar support in galeon and while fighting with the "what's related" panel I've found it would be very helpful to have this toplevel event target. I've got a workaround right now but this would allow things to work properly.
Reporter | ||
Updated•22 years ago
|
Target Milestone: mozilla1.1alpha → ---
Assignee | ||
Comment 5•20 years ago
|
||
I think this would be important for embedders. There is no other public way to connect chrome listeners right now. I think it could be considered a fix for bug 81835. Though DOMWindow is frozen now. Would it make sense to have nsIDOMWindow2.idl ?
Assignee | ||
Comment 6•20 years ago
|
||
Assignee | ||
Comment 7•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #148929 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #148930 -
Flags: superreview?(jst)
Attachment #148930 -
Flags: review?(bzbarsky)
Comment 8•20 years ago
|
||
I don't know when I'll be able to get to this review, to be truthful. I'm rather swamped with reviews and non-Mozilla work right now. :( I do wonder why we can't put this on nsIDOMWindowInternal (which isn't very internal).
Assignee | ||
Comment 9•20 years ago
|
||
(In reply to comment #8) > I don't know when I'll be able to get to this review, to be truthful. I'm > rather swamped with reviews and non-Mozilla work right now. :( OK. I'm cancelling it and ccing some other peers. Anyone has time to review this ? > I do wonder why we can't put this on nsIDOMWindowInternal (which isn't very > internal). Connecting chrome events is really a basic functionality for embedding. I dont think it's a good idea to expose it through an interface "which isn't very internal", whose name suggest it isnt really public and which is not planned to frozen. Obviously I'm not keen of nsIDOMWindow2 too, but I cant think of any other acceptable solution.
Assignee | ||
Updated•20 years ago
|
Attachment #148930 -
Flags: review?(bzbarsky)
Reporter | ||
Comment 10•20 years ago
|
||
Comment on attachment 148930 [details] [diff] [review] Add nsIDOMWindow2 with a windowRoot property (partially based on jst patch) nsIDOMWindowInternal is IMO not the answer here, nsIDOMWindow2 seems inevitable, so I'm sr'ing this change. It'll take some time before we can freeze this new interface tho, and before we do, we should probably pick through what's in nsIDOMWindowInternal and move over the methods/properties that we really do want embedders to use...
Attachment #148930 -
Flags: superreview?(jst) → superreview+
Comment 11•20 years ago
|
||
Comment on attachment 148930 [details] [diff] [review] Add nsIDOMWindow2 with a windowRoot property (partially based on jst patch) Okay, since jst is pleased, that's good enough for me. However: >+NS_IMETHODIMP >+GlobalWindowImpl::GetWindowRoot(nsIDOMEventTarget **aWindowRoot) >+{ >+ *aWindowRoot = nsnull; >+ >+ nsIDOMWindowInternal *rootWindow = GlobalWindowImpl::GetPrivateRoot(); >+ nsCOMPtr<nsPIDOMWindow> piWin(do_QueryInterface(rootWindow)); >+ if (!piWin) >+ return NS_OK; >+ >+ nsIChromeEventHandler *chromeHandler = piWin->GetChromeEventHandler(); >+ if (!chromeHandler) >+ return NS_OK; Please follow the preferred style which includes bracing even one line if-statements. >+ >+#include "nsIDOMWindow.idl" >+ >+[scriptable, uuid(65455132-b96a-40ec-adea-52fa22b1028c)] >+interface nsIDOMWindow2 : nsIDOMWindow >+{ >+ /** >+ * Get the window root for this window. This is useful for hooking >+ * up event listeners to this window and every other window nested >+ * in the window root. >+ */ >+ [noscript] readonly attribute nsIDOMEventTarget windowRoot; Would it make more sense to mark the interface as noscript if the functionality it provides is intended to be marked noscript? r=caillon
Attachment #148930 -
Flags: review+
Assignee | ||
Comment 12•20 years ago
|
||
Attachment #148930 -
Attachment is obsolete: true
Updated•20 years ago
|
Assignee: jst → marco
Comment 13•20 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•20 years ago
|
||
*** Bug 81835 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•20 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 15•20 years ago
|
||
Attachment #149121 -
Attachment is obsolete: true
Assignee | ||
Comment 16•20 years ago
|
||
Comment on attachment 149676 [details] [diff] [review] merge the uid change that was checked in I'd like to get this in 1.7. It's an important api for embedders and it would make the transition to embed string of GNOME users easier (GNOME 2.8 release is too near to be able to depend on mozilla 1.8). Also it should not be risky since it's just an addition and it didnt cause problems on the trunk.
Attachment #149676 -
Flags: approval1.7?
Comment 17•20 years ago
|
||
Comment on attachment 149676 [details] [diff] [review] merge the uid change that was checked in a=chofmann for 1.7
Attachment #149676 -
Flags: approval1.7? → approval1.7+
Assignee | ||
Updated•20 years ago
|
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Comment 19•20 years ago
|
||
this broke the 1.7 tree, did some API change? if so, are we certain it is safe for 1.7?
Assignee | ||
Comment 20•20 years ago
|
||
That's because nsPIDOMWindow is not de-xpcomified on 1.7 branch. I'll attach a patch in two minutes
Comment 21•20 years ago
|
||
biesi has fixed the bustage, he says it was due to deCOMtamination work on the trunk. can jst, caillon, and/or marco confirm that this patch should be nice and safe for 1.7?
Assignee | ||
Comment 22•20 years ago
|
||
I think it's safe. No API changed. There is only the addition of GetWindowRoot, which is unused in mozilla codebase. The build did break only because of the implementation of the unused API.
Whiteboard: fixed-aviary1.0
Comment 23•20 years ago
|
||
This patch added nsIDOMWindow2 to the Gecko SDK, but did not mark it frozen. That problem has been reported as bug 271295. In general, we should only add frozen interfaces to the Gecko SDK.
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•