Closed
Bug 1138290
Opened 9 years ago
Closed 9 years ago
Opens a new window at the connected external display
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
2.2 S11 (1may)
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: shelly, Assigned: shelly)
References
Details
Attachments
(1 file, 4 obsolete files)
7.81 KB,
patch
|
shelly
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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".
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8597108 -
Attachment is obsolete: true
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8597112 -
Attachment is obsolete: true
Assignee | ||
Comment 4•9 years ago
|
||
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 5•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8597113 -
Attachment is obsolete: true
Assignee | ||
Comment 7•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Attachment #8597770 -
Flags: review?(neil)
Assignee | ||
Comment 8•9 years ago
|
||
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+
Assignee | ||
Comment 10•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → slin
Assignee | ||
Comment 11•9 years ago
|
||
Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=64439264680d
Assignee | ||
Comment 12•9 years ago
|
||
Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b28218e9933
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/29894b01e4b8
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/29894b01e4b8
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S11 (1may)
You need to log in
before you can comment on or make changes to this bug.
Description
•