Opens a new window at the connected external display

RESOLVED FIXED in Firefox 40

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: shelly, Assigned: shelly)

Tracking

unspecified
2.2 S11 (1may)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Here we are trying to open a new 'toplevel' window at the connected external display. This can be achieved by WindowWatch.openWindow(), with some extra flags indicating it is opened via Wifidisplay or HDMI cord.

We can use 'moz-virtual-screen' for Wifidisplay and 'moz-external-screen' for HDMI cord.
Blocks: 1138269
This is one of the task from Bug 1116089 - Support "1 UA multi-screen" on FxOS. Please feel free to checkout the slide, demo video, wiki, and user stories from attachments of bug 1116089.

Details about this bug:

- We are trying to open a "top-level" window in b2g, for now, the one and only top-level window of b2g is opened by nsDefaultCLH.js with document of "shell.html" (https://dxr.mozilla.org/mozilla-central/source/toolkit/components/nsDefaultCLH.js#116)

So the steps of opening another top-level window to a specific connected display might look like:
(*)1. shell.js receives an 'display-changed' event, with a DisplayDevice object
DisplayDevice is an interface contains {displayId, connected, name}, work of shell.js is in bug 1156715, and work of DisplayDevice is in bug 1138287, for a closer look, please visit 
https://github.com/shellylin/gecko-dev/tree/multiscreen-v2.1

2. If connected==true, we will open a window with document of "shell-remote.html" from shell.js, using:

let options = 'chrome,dialog=no,close,resizable,scrollbars,extrachrome,' +
              'mozDisplayId='+aDisplayDevice.id;
let shellUrl = './shell-remote.html#' + aDisplayDevice.id;
let win = Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
          .getService(Components.interfaces.nsIWindowWatcher)
          .openWindow(null, shellUrl, 'myTopWindow' + aDisplayDevice.id, options, null);


This bug adds the extra options of "mozDisplayId=aDisplayId".
Attachment #8597108 - Attachment is obsolete: true
Attachment #8597112 - Attachment is obsolete: true
Comment on attachment 8597113 [details] [diff] [review]
New option in WindowWatcher.openWindow

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

Hi Michael,

We think that exposing and blindly using DisplayType everywhere is unnecessary.  This bug is for the part where we open a toplevel window with a DisplayId (instead of DisplayType) returned from gecko’s event. This Id is carried to the creation of nsWindow, so that a nsWindow can query its screen information through this unique Id (e.g. the screen bounds, native window, whether it can be composed by hwc…). 

DisplayId is generate by GonkDisplay when AddDisplay is called, and kept in this DisplayDevice as well. DisplayType is only needed when calling GonkDisplay::AddDisplay / RemoveDisplay.

Since we don’t have a device which allowed multiple displays of same type connected in the same time, DisplayId is equal to DisplayType for now.

See https://bug1138287.bugzilla.mozilla.org/attachment.cgi?id=8597207

I know this whole patch doesn’t seem to be related to your area, could you redirect to someone who would be available of reviewing? Thanks!
Attachment #8597113 - Flags: review?(mwu)
Comment on attachment 8597113 [details] [diff] [review]
New option in WindowWatcher.openWindow

Uhh, maybe Neil would be a good reviewer? https://wiki.mozilla.org/Modules/Core#XPApps

There's a lot of unrelated whitespace changes in here. Please remove them first since they'll be distracting to anyone reviewing this.
Attachment #8597113 - Flags: review?(mwu)
Attachment #8597113 - Attachment is obsolete: true
Comment on attachment 8597770 [details] [diff] [review]
New option in WindowWatcher.openWindow

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

Hi Neil, :mwu suggested you would be a good reviewer for this bug, if this is not true please just let me know.
For details about changes in this bug, please reference comment 1. Thanks!
Attachment #8597770 - Flags: review?(neil)
Attachment #8597770 - Flags: review?(neil)
Comment on attachment 8597770 [details] [diff] [review]
New option in WindowWatcher.openWindow

Hi roc, this is one of the task from project "multi-screen on b2g", nsWidgetInitData.h is modified to support the opening of multi-window, please see comment 1 for more details. Thanks :)
Attachment #8597770 - Flags: review?(roc)
Comment on attachment 8597770 [details] [diff] [review]
New option in WindowWatcher.openWindow

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

Looks reasonable. Please document that the display ID is platform-specific.
Attachment #8597770 - Flags: review?(roc) → review+
Blocks: 1138288
Carry r+ from roc, thanks!!

Nit fix in the patch:
- Documentation updated.
- Change display ID to screen ID for the reason of mScreenId is actually being used in widget/gonk/nsWindow, and this ID is designed for use by nsWindow. (see Bug 1138288)

Test case should be included and depends on bug 1138288.
Attachment #8597770 - Attachment is obsolete: true
Attachment #8599217 - Flags: review+
Assignee: nobody → slin
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/29894b01e4b8
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S11 (1may)
You need to log in before you can comment on or make changes to this bug.