After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 354181 - totem plugin should have a full screen option
totem plugin should have a full screen option
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: Browser plugin (obsolete)
2.16.x
Other Linux
: Normal enhancement
: ---
Assigned To: totem-browser-maint
totem-browser-maint
: 461020 (view as bug list)
Depends on:
Blocks: 458786
 
 
Reported: 2006-09-03 21:20 UTC by Sebastien Bacher
Modified: 2007-07-30 11:31 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
fullscreen option in windows media player plugin (37.48 KB, image/jpeg)
2006-09-05 03:05 UTC, Shaya Potter
  Details
Crude patch to make the plugin window fullscreen on middle click (1.80 KB, patch)
2007-06-24 16:20 UTC, Sunil Mohan Adapa
needs-work Details | Review
Patch that unsets the GTK_NO_WINDOW flag for bacon-video-widget (649 bytes, patch)
2007-06-26 11:15 UTC, Sunil Mohan Adapa
committed Details | Review
Patch to seperate most of the fullscreen code into totem-fullscreen.[ch] and split the glade file (67.48 KB, patch)
2007-06-28 12:44 UTC, Sunil Mohan Adapa
needs-work Details | Review
Patch to seperate most of the fullscreen code into totem-fullscreen.[ch] and split the glade file (updated to r4415) (56.89 KB, patch)
2007-06-29 07:02 UTC, Sunil Mohan Adapa
committed Details | Review
Patch to add fullscreen support in browser plugin (16.81 KB, patch)
2007-07-03 18:26 UTC, Sunil Mohan Adapa
none Details | Review
Patch to add fullscreen support in browser plugin (updated to r4430) (17.42 KB, patch)
2007-07-09 11:55 UTC, Sunil Mohan Adapa
needs-work Details | Review
Patch to add fullsceen support in browser plugin improved (17.42 KB, patch)
2007-07-23 19:24 UTC, Sunil Mohan Adapa
committed Details | Review

Description Sebastien Bacher 2006-09-03 21:20:21 UTC
That bug has been opened on https://launchpad.net/distros/ubuntu/+source/totem/+bug/56648

"The totem plugin should have an option to make it go full screen, much like other media player in plugin settings do.
...
> Thanks for your bug. What version of Ubuntu do you use? The edgy version has an "open totem" context menu action, do you consider it fixes your issue? I think it's easy enough to make totem go to fullscreen to no cluter the context menu with an extra option for that. What do you think?
...
oh and to answer your Q. if it did have an "open in totem" option, that might be good enough, unsure as one has to think of what functionality one wants to do from the plugin and I think 2 biggies are "save" and "go fullscreen" so making them an extra step, seems a waste.
...
I think fullscreen option would be a very good feature to have. For example a little icon appearing in top corner when you place your mouse over the video window. Average user expects something like this to happen, and if it doesn't happen, he/she thinks something's broken.
..."
Comment 1 Bastien Nocera 2006-09-03 21:27:16 UTC
Most of the time, fullscreening the plugin will suck. The movies are made to be embedded, not run fullscreen. So no fullscreen for the plugin.

Saving is bug #300691
And if the user is saying that some files are opened within the browser (but not embedded in a webpage, that would be bug #314370
Comment 2 Shaya Potter 2006-09-03 22:07:29 UTC
as the person who files the original bug in launchpad, I don't have any clue why you think this way.  I know that windows media player, real and quicktime plugins sometmes give you the option to go full screen (I think it's an option that can be passed to the plugin to allow the user the choice).  so I don't know why you'd dismiss this out of hand.
Comment 3 Shaya Potter 2006-09-05 03:05:57 UTC
Created attachment 72230 [details]
fullscreen option in windows media player plugin

As you can see, it's common for plugins to have full screen options, because there are many times that the video is provided at a rate good enough enough to view fullscreen.  For instance, Fox's streaming of prison break episodes.
Comment 4 Jonathan Métillon 2006-09-22 15:02:39 UTC
Hi Bastian,

I want to switch from MPlayer plugin to Totem plugin. Then I will be able to switch from Firefox to Epiphany, because Epiphany doesn't support MPlayer plugin. Totem plugin is built in Epiphany and there's no alternative.

But only MPlayer plugin supports full screen so I keep with it. Totem plugin must support full screen.

You said 3 weeks ago that there won't be any full screen option for the Totem Mozilla Plugin because:

1. "Most of the time, fullscreening the plugin will suck"
2. "The movies are made to be embedded, not run fullscreen"

Totem plugin mainteners have a huge responsability. This is GNOME default application for playing multimedia content on the web, be Firefox or Epiphany used. For sure newcomers will ask for full screen. This is very basic and needed feature. I can't believe you refuse to include it.

I don't know if full screen will suck with the plugin, I am not a Totem plugin developper. But I can affirm that full screen with Totem (not the plugin) doesn't suck at all. And I can affirm that full screen with MPlayer plugin doesn't suck at all neither. So I can't explain why full screen with Totem plugin would suck.

And no, movies are not *made* to be embedded. But the EMBED (deprecated) and OBJECT tags are made to embed multimedia files, and I don't see in the official HTML4 specifications about OBJECT(1) where it is said that videos should not be played full screen. Movies are made to be played full screen. Do you imagine windowed movies on tv or at the cinema? Why force users to watch small embeded videos when broadband Internet connection and high resolution screens enable them to experience high quality full screen movies online?

Full screen also increase accessibility. Some people can find difficult to watch small embeded videos, when being able to watch them in full screen will help a lot. Don't limit(2) GNOME user base, instead put the user in control!(3) :-) While looking at GNOME Human Interface Guidelines 2.0, I was not able to find a recommandation to disable full screen for embedded videos.

Sebastien Bacher proposed(4) one month ago to use a new "Open in Totem" context menu action featured in the future Edgy version of Ubuntu as an alternative. I also read(5)  that you already implemented that in CVS. No, I don't want to have to open an external application to be able to go full screen. We need an integrated full screen mode and a button inside the Totem plugin user interface.

I think that Shaya Potter is right and I hope to convince Totem plugin developpers to include a full screen feature as soon as possible, and to see this enhancement updated from WONTFIX to FIXED.  As you described yourself(6), you feel concerned by ease-of-use and accessibility, so I think it won't be hard to change your mind, even if you are experienced in strongly opiniated discussions and, as a French, which I am too, have a big mouth, which I have too.

I hope for your support Bastian. Thank you for your time.

(1) http://www.w3.org/TR/html4/struct/objects.html
(2) http://developer.gnome.org/projects/gup/hig/2.0/principles-broad-userbase.html
(3) http://developer.gnome.org/projects/gup/hig/2.0/principles-user-control.html
(4) https://launchpad.net/distros/ubuntu/+source/totem/+bug/56648
(5) http://www.hadess.net/ in "Totem Browser plugin work", 09 August 2006
(6) http://mail.gnome.org/archives/foundation-announce/2001-November/msg00002.html
Comment 5 Bastien Nocera 2006-09-22 15:07:57 UTC
Right, patches welcome.
Comment 6 Jonathan Métillon 2006-09-22 15:15:22 UTC
Thank you Bastien. I would like to help but I'm no GNOME developper. I Hope someone will be able to come with a patch!
Comment 7 Christian Persch 2006-09-22 15:24:05 UTC
(In reply to comment #4)
[...]
> because Epiphany doesn't support MPlayer
> plugin. Totem plugin is built in Epiphany and there's no alternative.

What do you mean by that? There is no reason mplayer plugin would not work in Epiphany. And totem-plugin is not 'built-in' to epiphany either.
Comment 8 Bastien Nocera 2006-10-08 15:57:56 UTC
*** Bug 360537 has been marked as a duplicate of this bug. ***
Comment 9 Javier Aravena 2006-12-05 23:15:55 UTC
I'm curious... will this feature ever exist?
Comment 10 Bastien Nocera 2006-12-06 00:30:33 UTC
(In reply to comment #9)
> I'm curious... will this feature ever exist?

2 things. First, what's the point of this update? Second, it will happen, but it's way down the list of things todo after a number of "making the plugin work better" bugs that need fixing.

If somebody wants to tackle this, code would need to be ripped out from Totem and shared between Totem and the totem-browser-viewer programs. It should make quite a bit of code common, and clean up totem.c some more.
Comment 11 Shaya Potter 2006-12-06 01:01:00 UTC
another reason to view it as an important feature is what I just hit in regards to some videos I placed behind some http auth.  If I do an "open in totem", totem has no concept of firefox's http auth, so needs to reauthenticate, not the biggest deal in the world, but a pain nonetheless.  If there was a "full screen" option from the plugin, presumambly, it still be using firefox for reading the file.
Comment 12 Sunil Mohan Adapa 2007-06-24 16:19:12 UTC
I need fullscreen for some work I am doing with Drupal. I (am new to totem and I) started to work on this and immediately got stuck because some part of the fullscreen video is showing blank. Can someone point to me what I am doing wrong, so I can try to work on this bug? I am attaching what I started on.
Comment 13 Sunil Mohan Adapa 2007-06-24 16:20:53 UTC
Created attachment 90564 [details] [review]
Crude patch to make the plugin window fullscreen on middle click
Comment 14 Sunil Mohan Adapa 2007-06-25 09:10:43 UTC
Looks like the problem is with bacon-video-widget. Does not correctly work with gtk_widget_reparent.
Comment 15 Christian Persch 2007-06-25 10:00:14 UTC
That's probably (part of) the same problem as bug 369134.
Comment 16 Sunil Mohan Adapa 2007-06-26 11:14:45 UTC
I think I got the reason for gtk_widget_reparent not working properly on the bacon-video-widget. GTK_NO_WINDOW flag was set on the widget. However, there are two GdkWindows (and consequently two X windows) for this widget. Once I unset this flag, reparenting is working fine and I see the plugin window in fullscreen. See the patch attached.

Now, I will try to implement fullscreen properly for the plugin.

Also, is there any particular reason BaconVideoWidget is derived from GtkBox instead of GtkWidget?
Comment 17 Sunil Mohan Adapa 2007-06-26 11:15:53 UTC
Created attachment 90656 [details] [review]
Patch that unsets the GTK_NO_WINDOW flag for bacon-video-widget
Comment 18 Sunil Mohan Adapa 2007-06-28 12:42:07 UTC
I seperated good part of the fullscreen code into totem-fullscreen.[ch] and split the glade file. I am attaching a patch.
Comment 19 Sunil Mohan Adapa 2007-06-28 12:44:22 UTC
Created attachment 90799 [details] [review]
Patch to seperate most of the fullscreen code into totem-fullscreen.[ch] and split the glade file
Comment 20 Bastien Nocera 2007-06-28 15:34:01 UTC
(In reply to comment #16)
> I think I got the reason for gtk_widget_reparent not working properly on the
> bacon-video-widget. GTK_NO_WINDOW flag was set on the widget. However, there
> are two GdkWindows (and consequently two X windows) for this widget. Once I
> unset this flag, reparenting is working fine and I see the plugin window in
> fullscreen. See the patch attached.

Nod, committed the patch now.

> Now, I will try to implement fullscreen properly for the plugin.
> 
> Also, is there any particular reason BaconVideoWidget is derived from GtkBox
> instead of GtkWidget?

Which version are you patching against? The current version descends from a GtkEventBox, because it matches best the current behaviour of the widget.

Sunil, could you update your patch to split the fullscreen code? Some code has changed in that regards in SVN trunk. You might also want to monitor bug 450635 and bug 451912 which affect this portion of the code.

Also, I'd rather the fullscreen mode was accessible from the right-click menu, and double-click on the video canvas (as currently used by Totem).

2007-06-28  Bastien Nocera  <hadess@hadess.net>

        * src/backend/bacon-video-widget-gst-0.10.c:
        (bacon_video_widget_init): patch from Sunil Mohan Adapa
        <sunilmohan@gnu.org.in> to not mark the video widget as having
        no backing window (Helps: #354181)
        * src/totem-private.h:
        * src/totem.c: (main): Remove unused ->fsvolume widget
Comment 21 Sunil Mohan Adapa 2007-06-28 16:06:51 UTC
(In reply to comment #20)
> > Now, I will try to implement fullscreen properly for the plugin.
> > 
> > Also, is there any particular reason BaconVideoWidget is derived from GtkBox
> > instead of GtkWidget?
> 
> Which version are you patching against? The current version descends from a
> GtkEventBox, because it matches best the current behaviour of the widget.

Aaaah! I asked that question and made the one-line patch against 2.18.2. I see that now it descends from GtkEventBox. However, the fullscreen split patch was against the trunk (at the time of submission :)

> 
> Sunil, could you update your patch to split the fullscreen code? Some code has
> changed in that regards in SVN trunk. You might also want to monitor bug 450635
> and bug 451912 which affect this portion of the code.

Ok, I'll update the patch.

> 
> Also, I'd rather the fullscreen mode was accessible from the right-click menu,
> and double-click on the video canvas (as currently used by Totem).
> 

Sure. I didn't yet start working on fullscreen for the plugin, but I will see that it works as you said (that crude middle click patch was more of a test case for the bug that halted me).

Thanks.
Comment 22 Sunil Mohan Adapa 2007-06-29 07:02:40 UTC
Created attachment 90847 [details] [review]
Patch to seperate most of the fullscreen code into totem-fullscreen.[ch] and split the glade file (updated to r4415)

I've update the fullscreen code splitting patch:

1) Merged with current svn.
2) I see that in the current svn, vol_fs_lock has been removed. vol_fs_lock was being used to prevent popup hiding. Popup hiding is not getting prevented correctly after removing this (esp. because of window active notifications). So, I retained it in my fullscreen seperation patch.
3) I have merged Jan's patch (id=90841) from bug #451912 since that patch conflicts with this one.

Philip's patch to move to GtkBuilder (id=90789) from bug #450635 also conflicts but it contains changes already in svn (like bacon-volume remove). So it needs to be redone.
Comment 23 Bastien Nocera 2007-06-29 09:41:31 UTC
I fixed a couple of bugs (totem_fullscreen_new takes a void, not "()").

I'm not sure the popdown avoidance actually works that well with the new volume widget, but I guess that's something for later.

2007-06-29  Bastien Nocera  <hadess@hadess.net>

        * POTFILES.in: upd

2007-06-29  Bastien Nocera  <hadess@hadess.net>

        * data/Makefile.am:
        * data/fullscreen.glade:
        * data/totem.glade:
        * src/Makefile.am:
        * src/totem-fullscreen.*:
        * src/totem-private.h:
        * src/totem.c: (totem_action_exit), (window_state_event_cb),
        (update_mrl_label), (totem_action_set_mrl_with_warning),
        (update_seekable), (update_current_time), (update_volume_sliders),
        (seek_slider_pressed_cb), (seek_slider_changed_cb),
        (seek_slider_released_cb), (totem_action_remote),
        (on_mouse_click_fullscreen), (totem_action_handle_scroll),
        (totem_callback_connect), (video_widget_create), (main):
        Patch from Sunil Mohan Adapa <sunilmohan@gnu.org.in> to split
        the fullscreen popup code into a separate widget (Helps: #354181),
        Patch from Jan Arne Petersen <jpetersen@jpetersen.org> to share
        adjustments between fullscreen and windowed seekbars (Closes: #451912)
Comment 24 Sunil Mohan Adapa 2007-07-03 18:26:36 UTC
Created attachment 91130 [details] [review]
Patch to add fullscreen support in browser plugin

I wrote a patch for plugin fullscreen.

* Double click on bvw (un)fullscreens.
* Menu item for fullscreen
* keyboard shortcuts in fullscreen mode: escape, f, F, F11, space, up, down
* uri for title label
* I could not test seeking completely.
* Some code (although not too much) had to copied from totem.c

This patch slightly clashes with Philip's Move to GtkBuilder patch (id=90882) from Bug #450635. If that goes in first, I will update this patch.
Comment 25 Sunil Mohan Adapa 2007-07-09 11:55:52 UTC
Created attachment 91490 [details] [review]
Patch to add fullscreen support in browser plugin (updated to r4430)

Updated the patch to apply against latest trunk.
Comment 26 Bastien Nocera 2007-07-23 10:52:15 UTC
Comment on attachment 91490 [details] [review]
Patch to add fullscreen support in browser plugin (updated to r4430)

<snip>
>+		fs_stock_id = GTK_STOCK_MEDIA_PLAY;
<snip>
<snip>
>+	gtk_tool_button_set_stock_id (GTK_TOOL_BUTTON (emb->pp_fs_button),
>+				      fs_stock_id);

fs_stock_id is the same as id in the end. Just use id instead.

<snip>
>+	} else if (event->type == GDK_2BUTTON_PRESS &&
>+		   event->button == 1) {
>+		totem_embedded_toggle_fullscreen (emb);

I tested double-click to go to fullscreen, and it didn't work very well for me. Could you remove this functionality for now?

<snip>
>+	case GDK_Up:
>+		totem_embedded_action_volume_relative (emb, VOLUME_UP_OFFSET);
>+		return TRUE;
>+	case GDK_Down:
>+		totem_embedded_action_volume_relative (emb, VOLUME_DOWN_OFFSET);
>+		return TRUE;

Couldn't we just send a "scroll-event" signal to the volume button instead?

Rest looks good, and ready to merge.
Comment 27 Sunil Mohan Adapa 2007-07-23 19:24:47 UTC
Created attachment 92224 [details] [review]
Patch to add fullsceen support in browser plugin improved

(In reply to comment #26)
[...]
> 
> fs_stock_id is the same as id in the end. Just use id instead.

Using "id" for both the buttons now. Tested RTL support with ar_IN.

[...]
> I tested double-click to go to fullscreen, and it didn't work very well for me.
> Could you remove this functionality for now?

Removed.

[...]
> 
> Couldn't we just send a "scroll-event" signal to the volume button instead?

I really didn't know how to do this. Wouldn't this mean creating a GdkEventScroll with a lot of ugly parameters? How about doing current_value +/- something? That would not eliminate those hard coded stepping constants though. (this constant exists in the main player also).

Thanks Bastien.
Comment 28 Bastien Nocera 2007-07-27 18:38:40 UTC
*** Bug 461020 has been marked as a duplicate of this bug. ***
Comment 29 Bastien Nocera 2007-07-30 11:31:04 UTC
2007-07-30  Bastien Nocera  <hadess@hadess.net>

        * browser-plugin/totem-plugin-viewer.c: (totem_embedded_finalize),
        (totem_embedded_set_state), (totem_embedded_set_pp_state),
        (totem_embedded_open_internal), (on_fullscreen1_activate),
        (totem_embedded_toggle_fullscreen),
        (totem_embedded_on_fullscreen_exit), (on_tick),
        (property_notify_cb_volume), (on_seek_start), (cb_on_seek),
        (totem_embedded_action_volume_relative),
        (totem_embedded_handle_key_press),
        (totem_embedded_key_press_event), (totem_embedded_construct):
        * data/mozilla-viewer.ui:
        * src/Makefile.am:
        * src/totem-fullscreen.c: (totem_fullscreen_new),
        (totem_fullscreen_set_video_widget):
        * src/totem-fullscreen.h:
        * src/totem.c: (main): Patch from Sunil Mohan Adapa
        <sunilmohan@gnu.org.in> to add a fullscreen option to the
        browser plugin (Closes: #354181)

Thanks for the work Sunil!