Closed
Bug 1060191
Opened 10 years ago
Closed 10 years ago
Web compat issues with window.open
Categories
(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.2 fixed)
People
(Reporter: kgrandon, Assigned: gduan)
References
Details
Attachments
(4 files)
Currently the way gaia opens new windows has many issues for web compatibility. Here are a few known issues: - It's likely that we will open the same URL or named window twice. - We do not open windows in the same process. - We probably do not have a proper reference to window.opener.
Reporter | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #0) > Currently the way gaia opens new windows has many issues for web > compatibility. Here are a few known issues: > > - It's likely that we will open the same URL or named window twice. > - We do not open windows in the same process. > - We probably do not have a proper reference to window.opener. I don't understand the problem here. The window names are tracked in Gecko and Gecko will silently re-use the popup window in the same process to load the new content. So this should work and system app should only receive mozbrowseropenwindow once: var win = window.open('https://www.mozilla.org/#foo', 'moz'); var win2 = window.open('https://www.mozilla.org/#foo2', 'moz'); console.log(win === win2); // true console.log(win2.location.href); // will be "https://www.mozilla.org/#foo2" assuming the frame is loaded I think the problem only arise if we, instead of appending the passed frame element for the event to the DOM, we ditch it and do something else. In that case |win| will be null (since no "window" was created from the app's perspective). Is that what we are doing right now? What do we do in Gaia to break the web?
Reporter | ||
Comment 3•10 years ago
|
||
I think the issue may only be related to windows with a name of _blank. I believe in this case the window.opener object should exist? Here is a pull request which adds a failing test case. The window.opener object is null in the opened window.
Comment 4•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #2) > (In reply to Kevin Grandon :kgrandon from comment #0) > > Currently the way gaia opens new windows has many issues for web > > compatibility. Here are a few known issues: > > > > - It's likely that we will open the same URL or named window twice. > > - We do not open windows in the same process. > > - We probably do not have a proper reference to window.opener. > > I don't understand the problem here. The window names are tracked in Gecko > and Gecko will silently re-use the popup window in the same process to load > the new content. So this should work and system app should only receive > mozbrowseropenwindow once: > > var win = window.open('https://www.mozilla.org/#foo', 'moz'); > var win2 = window.open('https://www.mozilla.org/#foo2', 'moz'); > > console.log(win === win2); // true > console.log(win2.location.href); // will be "https://www.mozilla.org/#foo2" > assuming the frame is loaded > > I think the problem only arise if we, instead of appending the passed frame > element for the event to the DOM, we ditch it and do something else. In that > case |win| will be null (since no "window" was created from the app's > perspective). > > Is that what we are doing right now? What do we do in Gaia to break the web? What Kevin says. This happens specifically because of this code: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/child_window_factory.js#L57 The issue is that instead of reusing the window passed in the detail object of mozbrowseropenwindow, the Gaia codebase speficially open a new AppWindow, and load the asked url into it. I think this issue is here for system bookmarks / e.me collection for a while, but we didn't pay attention to it, as it was working correctly in the browser app. Now that the browser app is moved back into the system app, this is much more painful as it may results into unexpected results for web site.
Comment 5•10 years ago
|
||
[Blocking Requested - why for this release]: Web Compatibility with window.open('foo.html', '_blank') is broken, and may results into unexpected web breakage...
blocking-b2g: --- → 2.1?
Updated•10 years ago
|
Assignee: nobody → alive
blocking-b2g: 2.1? → 2.1+
Updated•10 years ago
|
Target Milestone: --- → 2.1 S5 (26sep)
Assignee | ||
Updated•10 years ago
|
Assignee: alive → gduan
Flags: needinfo?(gduan)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8490228 [details] [review] PR to master Hi Alive, could you review this patch? Thanks.
Attachment #8490228 -
Flags: review?(alive)
Comment 9•10 years ago
|
||
Comment on attachment 8490228 [details] [review] PR to master f+, I'd like to let AWF to create all AppWindow instances.
Attachment #8490228 -
Flags: review?(alive) → feedback+
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8490228 [details] [review] PR to master Hi Alive, could you take a look on this patch? Thanks.
Attachment #8490228 -
Flags: review?(alive)
Comment 11•10 years ago
|
||
Comment on attachment 8490228 [details] [review] PR to master r+ with nits
Attachment #8490228 -
Flags: review?(alive) → review+
Assignee | ||
Comment 12•10 years ago
|
||
PR to master https://github.com/mozilla-b2g/gaia/commit/6163c5ee0f99f73439318d064e0862ccd4e11a49
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 13•10 years ago
|
||
Please add a Gaia v2.1 nomination when you get a chance :)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8496650 [details] [review] PR to 2.1 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): No. [User impact] if declined: As Comment 5, window.open('foo.html', '_blank') may result into unexpected web breakage if some page use it. [Testing completed]: Yes. [Risk to taking this patch] (and alternatives if risky): No. [String changes made]:
Attachment #8496650 -
Flags: approval-gaia-v2.1?
Updated•10 years ago
|
Attachment #8496650 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Comment 16•10 years ago
|
||
v2.1: https://github.com/mozilla-b2g/gaia/commit/e9a2f4dc71025d2f4d136a9ce765bf0aa74d018d
Depends on: 1078707
Comment 17•10 years ago
|
||
Hello Kevin, Would you please offer me a full step, and please tell which expression is fail. Thanks!
Flags: needinfo?(kgrandon)
Reporter | ||
Comment 18•10 years ago
|
||
(In reply to SandKing from comment #17) > Hello Kevin, > Would you please offer me a full step, and please tell which expression is > fail. > Thanks! I'm not sure I understand the question here. If you want to verify the bug, you'll likely need to create a test page, or find one which references the window.opener property after a window.open().
Flags: needinfo?(kgrandon)
You need to log in
before you can comment on or make changes to this bug.
Description
•