Closed Bug 1116860 Opened 5 years ago Closed 5 years ago

Get size of second screen from the chromecast

Categories

(Firefox for Android :: Screencasting, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 38
Tracking Status
fennec 36+ ---

People

(Reporter: blassey, Assigned: blassey)

References

Details

Attachments

(2 files)

No description provided.
Attachment #8543071 - Flags: review?(mark.finkle)
Assignee: nobody → blassey.bugs
Attachment #8543072 - Flags: review?(mark.finkle)
Duplicate of this bug: 1113715
Comment on attachment 8543071 [details] [diff] [review]
get_size_from_chromecast.patch

>diff --git a/mobile/android/modules/TabMirror.jsm b/mobile/android/modules/TabMirror.jsm

>   Services.obs.addObserver((aSubject, aTopic, aData) => this._processMessage(aData), "MediaPlayer:Response", false);
>-
>+  this._sendMessage({start: true});

add spaces:
{ start: true }

> TabMirror.prototype = {

>+  _screenSize: {width: 1280, height: 720},

add spaces here too

>+  _start: function() {

>+    let windowId = this._window.BrowserApp.selectedBrowser.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils).outerWindowID;
>+    let viewport = this._window.BrowserApp.selectedTab.getViewport();

if _window is null, bad things will happen. I don't suppose we have any alternative though.

>+    log("viewport: " + JSON.stringify(viewport));

Remove?

>   _processMessage: function(data) {

>+    if (msg.sdp && msg.type === "answer") {
>+      this._processAnswer(msg);
>+    } else if (msg.candidate) {
>+      this._processIceCandidate(msg);
>+    } else if (msg.height || msg.width) {

It's kinda nice that msg.type exists for "answer". Using the presence of | width | or | height | might lead to future bugs. How hard would it be to add | msg.type = "size" |?

Looks OK. Think about the msg.type thing.

Asking Randall to take a look too.
Attachment #8543071 - Flags: review?(mark.finkle)
Attachment #8543071 - Flags: review+
Attachment #8543071 - Flags: feedback?(rbarker)
Comment on attachment 8543072 [details] [diff] [review]
chromecast_send_size.patch

>diff --git a/player.js b/player.js

>+      this._sendMessage({width: window.innerWidth, height: window.innerHeight});

add spaces around the curlies

{ width ... }

Remember to add | type: "size", | if you decide to use it.
Attachment #8543072 - Flags: review?(mark.finkle) → review+
> Asking Randall to take a look too.

Look okay to me.
Attachment #8543071 - Flags: feedback?(rbarker)
tracking-fennec: ? → 36+
https://hg.mozilla.org/mozilla-central/rev/a88e871c1dfc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
You need to log in before you can comment on or make changes to this bug.