Closed Bug 1557407 Opened 5 years ago Closed 4 years ago

Don't expose the `DOMWindowClosed` event to content

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: nika, Assigned: u608768)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file, 1 obsolete file)

It's a non-standard internal event which we're (AFAICT) accidentally exposing to content. Chrome does not fire an event like this.

Actual:

>> document.body.innerHTML += `<button onclick="_win = window.open()">clicky</button>`;
>> _win.addEventListener("DOMWindowClose", function(e) { console.log("DOMWindowClose"); console.log(e); });
>> console.log(_win.closed); _win.close(); console.log(_win.closed);
false
DOMWindowClose
DOMWindowClose { target: Window, isTrusted: true, srcElement: Window, currentTarget: Window, eventPhase: 2, bubbles: true, cancelable: true, returnValue: true, defaultPrevented: false, composed: false, … }
true

Expected:

>> document.body.innerHTML += `<button onclick="_win = window.open()">clicky</button>`;
>> _win.addEventListener("DOMWindowClose", function(e) { console.log("DOMWindowClose"); console.log(e); });
>> console.log(_win.closed); _win.close(); console.log(_win.closed);
false
true

The root of the problem here is that this event is dispatched using the convenient nsGlobalWindowOuter::DispatchCustomEvent interface, which doesn't mark the events dispatched using it as chrome-only. The callers of this appear to be ones which expect this behaviour, but some may not.

The events dispatched using this method this include:

  • willenterfullscreen
  • willexitfullscreen
  • fullscreen
  • DOMWindowClose
  • XULAlertClose
  • sizemodechange
  • resolutionchange
  • orientationchange
  • occlusionstatechange

A quick scan suggests that DOMWindowClose and XULAlertClose should perhaps be restricted to chrome-only.

ni? smaug for thoughts on this.

Flags: needinfo?(bugs)

yeah, we should dispatch only the standard events to content.

Flags: needinfo?(bugs)

Add an enum arg to DispatchCustomEvent for whether it should go to content?

Adds an aChromeOnlyDispatch flag to DispatchCustomEvent to determine which
Dispatch method to call (DispatchTrustedEvent or DispatchEventOnlyToChrome).

The following chrome-only events were previously dispatched with
DispatchCustomEvent and now make use of the flag:

DOMWindowClose
fullscreen / willenterfullscreen / willexitfullscreen (content already don't
get these, not entirely sure why)
occlusionstatechange
resolutionchange
XULAlertClose

and the following were previously dispatched with DispatchTrustedEvent and now
use DispatchEventOnlyToChrome:

MozBeforeInitialXULLayout
MozMouseScrollFailed
MozMouseScrollTransactionTimeout
MozPaintWaitFinished / MozPaintWait
MozPerformDelayedBlur
all events fired by APZCCallbackHelper::NotifyMozMouseScrollEvent

Adds an |aChromeOnlyDispatch| flag to DispatchCustomEvent to determine which
Dispatch method to call (DispatchTrustedEvent or DispatchEventOnlyToChrome).

The following chrome-only events were previously dispatched with
DispatchCustomEvent and now make use of the flag:

  • DOMWindowClose
  • fullscreen / willenterfullscreen / willexitfullscreen
  • occlusionstatechange
  • resolutionchange
  • XULAlertClose

and the following were previously dispatched with DispatchTrustedEvent and now
use DispatchEventOnlyToChrome:

  • MozBeforeInitialXULLayout
  • MozMouseScrollFailed
  • MozMouseScrollTransactionTimeout
  • MozPaintWaitFinished / MozPaintWait
  • MozPerformDelayedBlur
  • all events fired by APZCCallbackHelper::NotifyMozMouseScrollEvent
Assignee: nobody → kmadan
Status: NEW → ASSIGNED
Attachment #9078865 - Attachment is obsolete: true
Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7a1572c41c2
Replace DispatchTrustedEvent with DispatchEventOnlyToChrome for various chrome-only events, r=smaug
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

Posted a site compatibility note for the change.

Keywords: site-compat

I assume we don't document these events on MDN, but we should check and remove the docs if needed.

Keywords: dev-doc-needed

I've added a rel note to cover this: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#APIs

I've checked MDN, and none of these events are mentioned, so I don't think there's any more to do here.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: