Closed Bug 821073 Opened 7 years ago Closed 7 years ago

full screen support for video chats

Categories

(Firefox Graveyard :: SocialAPI, defect)

x86
macOS
defect
Not set

Tracking

(firefox23 verified, firefox24 verified)

VERIFIED FIXED
Firefox 24
Tracking Status
firefox23 --- verified
firefox24 --- verified

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

Details

Attachments

(1 file, 2 obsolete files)

need to look into supporting moving chat windows into fullscreen mode
Attached patch exploratory patch (obsolete) — Splinter Review
this is just a quick hack patch to move into full screen if you're not a browser tab, lots of stuff still doesn't work with a video tag in a chat window.
Not directly related to the determination for if we ship webrtc or not.
Whiteboard: [webrtc]
Attached patch fullscreen patch (obsolete) — Splinter Review
full screen patch allows content in chat windows to go full screen.  relies on css for full screen mode.  also replaces previous work on hiding social when browser tabs go full screen by relying on css, resulting in a better fullscreen transition.
Assignee: nobody → mixedpuppy
Attachment #691559 - Attachment is obsolete: true
Attachment #755005 - Flags: review?(felipc)
Comment on attachment 755005 [details] [diff] [review]
fullscreen patch

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

::: browser/base/content/browser-fullScreen.js
@@ +107,5 @@
>      // However, if we receive a "MozEnteredDomFullScreen" event for a document
>      // which is not a subdocument of the currently selected tab, we know that
>      // we've switched tabs since the request to enter full-screen was made,
>      // so we should exit full-screen since the "full-screen document" isn't
>      // acutally visible.

just need to update this comment with something like
s/which is not a subdocument of the currently selected tab/which is not active/

@@ +110,5 @@
>      // so we should exit full-screen since the "full-screen document" isn't
>      // acutally visible.
> +    if (!event.target.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
> +                                 .getInterface(Ci.nsIWebNavigation)
> +                                 .QueryInterface(Ci.nsIDocShell).isActive) {

I wonder here if a subdocument can be QI'ed to webNav/docShell and isActive works, or if you'll need to use .top
Attachment #755005 - Flags: review?(felipc) → review+
(In reply to :Felipe Gomes from comment #4)
> >      // so we should exit full-screen since the "full-screen document" isn't
> >      // acutally visible.
> > +    if (!event.target.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
> > +                                 .getInterface(Ci.nsIWebNavigation)
> > +                                 .QueryInterface(Ci.nsIDocShell).isActive) {
> 
> I wonder here if a subdocument can be QI'ed to webNav/docShell and isActive
> works, or if you'll need to use .top

isActive state is propagated from parent to child docshells: http://hg.mozilla.org/mozilla-central/annotate/4755d50e2402/docshell/base/nsDocShell.cpp#l5236
Attached patch fullscreen patchSplinter Review
patch with fixed comment

https://hg.mozilla.org/integration/mozilla-inbound/rev/d70b4807df13
Attachment #755005 - Attachment is obsolete: true
Attachment #757014 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/d70b4807df13
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Comment on attachment 757014 [details] [diff] [review]
fullscreen patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): chat windows
User impact if declined: webrtc video chats do not work well in the small confines of the text chat window, video is basically unusable at that size.  this allows for video calls to go full screen
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #757014 - Flags: approval-mozilla-aurora?
Attachment #757014 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This has been on central for a couple of weeks but adding verifyme to make sure this works as expected before we get to Beta on Monday.
Keywords: verifyme
QA Contact: srgohilgnfc
I have tested this using demo provider from http://mixedpuppy.github.io/socialapi-demo/
on Windows 8.

User Agent:Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
user Id ID: 20130625125232

Video works in full screen.

Is it sufficient to verify using the video embedded in the chat panel of the demo provider?
Flags: needinfo?(mixedpuppy)
(In reply to Samvedana from comment #12)
> I have tested this using demo provider from
> http://mixedpuppy.github.io/socialapi-demo/
> on Windows 8.
> 
> User Agent:Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101
> Firefox/23.0
> user Id ID: 20130625125232
> 
> Video works in full screen.
> 
> Is it sufficient to verify using the video embedded in the chat panel of the
> demo provider?

For this bug, yes.  fullscreen for tear-off chat windows (bug 880911) is slightly different testing but can also be tested with the demo provider.
Flags: needinfo?(mixedpuppy)
(In reply to Shane Caraveo (:mixedpuppy) from comment #13)
> (In reply to Samvedana from comment #12)
> > I have tested this using demo provider from
> > http://mixedpuppy.github.io/socialapi-demo/
> > on Windows 8.
> > 
> > User Agent:Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101
> > Firefox/23.0
> > user Id ID: 20130625125232
> > 
> > Video works in full screen.
> > 
> > Is it sufficient to verify using the video embedded in the chat panel of the
> > demo provider?
> 
> For this bug, yes.  fullscreen for tear-off chat windows (bug 880911) is
> slightly different testing but can also be tested with the demo provider.

Thanks Shane!
Status: RESOLVED → VERIFIED
Samvedana, can you please retest this against Firefox 24?
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:24.0) Gecko/20130723 Firefox/24.0
Build ID: 20130723004004

I have tested this using demo provider from http://mixedpuppy.github.io/socialapi-demo/
on Windows 8.

Video works in full screen.
See Also: → 1199519
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.