If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Find a way to proxy window.content

RESOLVED FIXED in mozilla25

Status

()

Core
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla25
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Quite a few addons use window.content in chrome. I think we need to somehow give them window.contentWindow, which already works with CPOWs.
Worth pointing out that it's not only add-ons, there's quite a lot of code in browser.js and elsewhere that uses |window.content| or, more commonly, just |content|
(Assignee)

Comment 2

4 years ago
I found a way to do this. Don't judge me.
https://hg.mozilla.org/projects/larch/rev/dfa541dbaf3b
(Assignee)

Comment 3

4 years ago
Nicer and cleaner way as recommended by bz.
https://hg.mozilla.org/projects/larch/rev/b23a59e25225
(Assignee)

Comment 4

4 years ago
Now that CPOWs landed on central, I am going to start on making these patches land-able.
(Assignee)

Comment 5

4 years ago
Created attachment 775941 [details] [diff] [review]
patch for webProgress and contentWindow

This patch adds browser.contentWindow, contentDocument and webProgess.DOMWindow.
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #775941 - Flags: review?(felipc)
(Assignee)

Comment 6

4 years ago
Created attachment 775943 [details] [diff] [review]
refreshed

D'uh has been happening a lot lately.
Attachment #775941 - Attachment is obsolete: true
Attachment #775941 - Flags: review?(felipc)
Attachment #775943 - Flags: review?(felipc)
(Assignee)

Comment 7

4 years ago
Created attachment 776085 [details] [diff] [review]
the actual window.content patch.

This is redefining |content| to return a jsval, because otherwise we couldn't return a CPOW. I am not sure if there is some way to only do this for chrome JS code, but I suspect not. So we end up doing the normal thing in GetContentScriptable, but if we are trying to get content on a chrome window from chrome code, we end up calling a new method of nsIBrowserDOMWindow, that just returns the current browsers contentWindow. This surprised me at first, but apparently there is only one of these chrome windows or something like that. I don't usually mess around with this code, so I hope it's not too bad.
Attachment #776085 - Flags: review?(bzbarsky)
Comment on attachment 776085 [details] [diff] [review]
the actual window.content patch.

>+++ b/dom/base/nsGlobalWindow.cpp
>+nsGlobalWindow::GetScriptableContent(JSContext* aCx, JS::Value* aVal)
>+      JS::Rooted<JS::Value> val(aCx);

This seems to be unused.

>+++ b/dom/interfaces/base/nsIBrowserDOMWindow.idl
>+  jsval getContentWindow();

Is there a reason this is not a |readonly attribute jsval contentWindow;|?

r=me with those addressed
Attachment #776085 - Flags: review?(bzbarsky) → review+
Comment on attachment 775943 [details] [diff] [review]
refreshed

Review of attachment 775943 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/browser-child.js
@@ +33,5 @@
> +    let win = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
> +                      .getInterface(Ci.nsIDOMWindow);
> +    return {
> +      contentWindow: win,
> +      DOMWindow: aWebProgress.DOMWindow // DOMWindow is not necessarily the content-window.

can you explain a case where win != DOMWindow? is it for subframes?
Attachment #775943 - Flags: review?(felipc) → review+
(Assignee)

Comment 10

4 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/6a788f399a7f
http://hg.mozilla.org/integration/mozilla-inbound/rev/c15f503a96cc
Something in this push was causing Linux debug mochitest-other to be perma-crashing. I couldn't find you on IRC, so I had to back the entire push out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/998c273658b8

https://tbpl.mozilla.org/php/getParsedLog.php?id=25380532&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=25380601&tree=Mozilla-Inbound
(Assignee)

Comment 12

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ebd354a200a
(Assignee)

Comment 13

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3991732978d1
https://hg.mozilla.org/mozilla-central/rev/8ebd354a200a
https://hg.mozilla.org/mozilla-central/rev/3991732978d1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

4 years ago
Depends on: 902013
You need to log in before you can comment on or make changes to this bug.