1192489 - prevent Gecko's default window.open handler on mozbrowseropenwindow

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Over in bug 1135261, I'm changing BrowserElementParent to respect the default status of mozbrowseropenwindow (and mozbrowserasyncscroll) events, so Gecko invokes its default handler for those events unless Event.preventDefault() has been called.  See bug 1135261, comment 37 for the details.

That change means that Gaia will need to start calling preventDefault() when handling mozbrowseropenwindow (and perhaps also mozbrowserasyncscroll).

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] mykmelez:window-open-prevent-default > mozilla-b2g:master

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Tim: Are you the right reviewer for this?  https://wiki.mozilla.org/Modules/FirefoxOS#System lists you as an owner of the System module, but please reassign this if someone else should look at it!

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8645305 [details] [review]
[gaia] mykmelez:window-open-prevent-default > mozilla-b2g:master

This might be the right place to call the |preventDefault()|. However, I am not entire sure this is the right API change, or a safe API change. Can Gecko simply figure out the same information by looking at the iframe element instance and see if it's appended on the parent document?

Comment 4

[Myk Melez [:myk] [@mykmelez]]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #3)
> Comment on attachment 8645305 [details] [review]
> [gaia] mykmelez:window-open-prevent-default > mozilla-b2g:master
> 
> This might be the right place to call the |preventDefault()|. However, I am
> not entire sure this is the right API change, or a safe API change.

The DOM Event model describes preventDefault as the method "used to signify that the event is to be canceled, meaning any default action normally taken by the implementation as a result of the event will not occur." <http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-Event-preventDefault>

And BrowserElementParent::DispatchOpenWindowEvent claims to "report to callers whether preventDefault was called on or not." <http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementParent.cpp?rev=98c2f8a27530#196>

But bug 1135261 means that it always reports that preventDefault is called, whether or not it actually is.  And that breaks other consumers of the API, like the desktop runtime.  So the API change in bug 1135261 seems correct to me, since it makes the API do what it actually claims to be doing, and it makes it behave the same way as other DOM events.

As for being a safe API change, it's possible that there are other apps using the Browser API that will need to be updated.  But there are relatively few consumers of that API, and it still seems better to fix the bug than perpetuate it.


> Can Gecko simply figure out the same information by looking at the iframe
> element instance and see if it's appended on the parent document?

Gecko could do that, but it wouldn't always work correctly, since sometimes a browser will want to ignore a window.open call (because the app shouldn't be able to open the window), which means it won't append the iframe to the parent document.  But it also won't want Gecko to take its default action.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/d618d895450d821e1ca7e73828a1404ed20ce9a0