Launchpad has imported 28 comments from the remote bug at https://bugzilla.mozilla.org/show_bug.cgi?id=811261.
If you reply to an imported comment from within Launchpad, your comment will be sent to the remote bug automatically. Read more about Launchpad's inter-bugtracker facilities at https://help.launchpad.net/InterBugTracking. ------------------------------------------------------------------------ On 2012-11-13T11:34:43+00:00 Stransky wrote: +++ This bug was initially created as a clone of Bug #517870 +++ User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11; .NET CLR 2.0.50727; ffco7) Gecko/2009060215 Firefox/3.0.11 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11; .NET CLR 2.0.50727; ffco7) Gecko/2009060215 Firefox/3.0.11 During playback, fullscreen video should disable the screensaver to allow watching arbitrarily long videos uninterrupted. This is a feature that is common in native player and would be a leg up on flash-based players. Reproducible: Always There is a registry key that could be used for this purpose on Windows, but I'm not sure I like it for the same reasons given here: http://mailman.videolan.org/pipermail/vlc-devel/2008-December/054231.html but the workaround they came up with, while perhaps a bit hacky, would work fine here too. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/37 ------------------------------------------------------------------------ On 2012-11-14T12:43:26+00:00 Stransky wrote: Created attachment 681449 WIP WIP. Unfortunately the D-Bus interface seems to be changed so the Inhibit method does not work. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/38 ------------------------------------------------------------------------ On 2012-11-14T15:00:09+00:00 Stransky wrote: Created attachment 681480 v1 A working one, via. Session Manager & D-Bus. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/39 ------------------------------------------------------------------------ On 2012-11-21T04:45:48+00:00 Karlt wrote: Comment on attachment 681480 v1 IIUC this will inhibit the screensaver, even if there is an advert or audio playing out of view on any foreground tab of an unminimized window. I'm not sure it is reasonable to require users to minimize their windows to allow the screensaver to activate. Perhaps there is some other mechanism to prevent media playing until the user interacts. Better get feedback from the media team on this. Does Gnome Shell let you minimize windows? Does it unmap windows on background virtual desktops? Don't know whether only considering the active window would be better. On Android, I assume it doesn't make much difference because only one window will be visible at a time. At least considering only active windows would filter out windows on other virtual desktops. But the same question still remains about whether leaving an active window visible should allow the site to disable the screensaver. Seems we should only be disabling the screensaver on some user action, such as asking for (or allowing fullscreen). Does the screensaver that you work with support org.freedesktop.ScreenSaver? That sounds like the preferred API to use, but I haven't found official documentation. It seems very similar to org.gnome.ScreenSaver. http://lists.freedesktop.org/archives/xdg/2007-March/009187.html Using a GNOME interface is OK if the freedesktop interface is not supported and using the GNOME interface doesn't cause any new processes to start (in sessions with different managers). What is the reason to prefer gnome.SessionManager over gnome.ScreenSaver? reason = "Fullscreen mode" isn't matching up with the conditions of calling Inhibit. I agree fullscreen mode seems the more appropriate condition under which to disable the screensaver. We don't want to wait for the DBus server when starting to play, so the calls should be asynchronous. Blocking on the inhibit reply before uninhibit should be OK. When not using dbus_connection_send_with_reply_and_block, something will be necessary to actually flush the dbus send queue. dbus_connection_flush() doesn't seem quite ideal because it "blocks until it can write out the entire outgoing queue." I'm guessing dbus-glib's dbus_connection_setup_with_g_main will be the easiest way to look after that. We perhaps could get lucky that dbus_connection_setup_with_g_main has already been called elsewhere, but better to ensure it is called by adding another call here. Feel free to use other dbus-glib API if that makes things simpler. I don't really mind whether libdbus or dbus-glib functions are used because both are superseded or deprecated by GIO's GDBus, and GDBus is too new to use. The concept of activating a WakeLock on notification that the foreground has already been locked seems quite strange to me, but it does follow the Android implementation. Best to get feedback from the author or reviewer of those changes because there are some things that I don't understand: There doesn't seem to be any documentation on what a Wakelock is for, what the topic is used for, or the difference between hidden and active locks. The android-specific listener provides for concurrent locks of different topic/tag. Why is ModifyWakeLock implemented in the hardware abstraction layer when it doesn't interact with the hardware? Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/40 ------------------------------------------------------------------------ On 2012-11-27T15:16:16+00:00 Stransky wrote: (In reply to Karl Tomlinson (:karlt) from comment #3) > IIUC this will inhibit the screensaver, even if there is an advert or audio > playing out of view on any foreground tab of an unminimized window. I tested it and it inhibits the screensaver in fullscreen mode only? Why do you think it affects non-fullscreen playback too? > Does the screensaver that you work with support org.freedesktop.ScreenSaver? > That sounds like the preferred API to use, but I haven't found official > documentation. It seems very similar to org.gnome.ScreenSaver. > http://lists.freedesktop.org/archives/xdg/2007-March/009187.html org.freedesktop.ScreenSaver is outdated and does not work. The SessionManager is used by gnome itself (by http://git.gnome.org/browse/gtk+/tree/gtk/gtkapplication.c#n1412). Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/42 ------------------------------------------------------------------------ On 2012-11-27T15:18:12+00:00 Stransky wrote: Sorry, I mean it inhibits the screen saver only in fullscreen, not when the video is embeded. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/43 ------------------------------------------------------------------------ On 2012-11-27T20:17:52+00:00 Karlt wrote: (In reply to Martin Stránský from comment #4) > (In reply to Karl Tomlinson (:karlt) from comment #3) > > IIUC this will inhibit the screensaver, even if there is an advert or audio > > playing out of view on any foreground tab of an unminimized window. > > I tested it and it inhibits the screensaver in fullscreen mode only? Why do > you think it affects non-fullscreen playback too? The code added in bug 739542 looks like it adds a WakeLock with a "Playing_media" tag whenever an nsHTMLMediaElement is not paused. I'm assuming nsHTMLMediaElement is used for non-fullscreen media. > > Does the screensaver that you work with support org.freedesktop.ScreenSaver? > > That sounds like the preferred API to use, but I haven't found official > > documentation. It seems very similar to org.gnome.ScreenSaver. > > http://lists.freedesktop.org/archives/xdg/2007-March/009187.html > > org.freedesktop.ScreenSaver is outdated ... KDE people may contest that. It would seem strange to switch from a freedesktop interface to a gnome interface. > ... and does not work. But if org.freedesktop.ScreenSaver doesn't work we do need something else. > The SessionManager is used by gnome itself (by > http://git.gnome.org/browse/gtk+/tree/gtk/gtkapplication.c#n1412). It is a shame that GTK3 now depends on GNOME. But this does look like evidence that org.gnome.SessionManager is preferred over org.gnome.ScreenSaver, thanks. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/44 ------------------------------------------------------------------------ On 2012-11-28T10:39:46+00:00 Stransky wrote: > But if org.freedesktop.ScreenSaver doesn't work we do need something else. Ahh, sorry. org.gnome.ScreenSaver is outdated and does not work. org.freedesktop.ScreenSaver seems to run but on KDE only, it's not exposed by GTK/Gnome. So I'm not sure how to implement it, because KDE uses org.freedesktop.ScreenSaver and org.gnome.SessionManager. It's shame that org.freedesktop.ScreenSaver is not implemented by Gnome. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/45 ------------------------------------------------------------------------ On 2012-11-28T13:03:07+00:00 ChristianSchaller wrote: I checked around a bit. So there is no 'freedesktop' interface. The so called freedesktop interface was a proposal from some KDE developers that never got adopted as a freedesktop standard, but KDE kept 'freedesktop' in their DBus API name which one could argue they shouldn't have done. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/46 ------------------------------------------------------------------------ On 2012-11-28T18:13:44+00:00 Bastien Nocera wrote: I've implemented the "freedesktop"[1] interface in GNOME, it will be available for GNOME 3.6 (the current version) and later[2]. Please use this (the inhibit/uninhibit calls) to implement the screensaver inhibition in Firefox. [1]: https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/master/entry/ksmserver/screenlocker/dbus/org.freedesktop.ScreenSaver.xml [2]: https://bugzilla.gnome.org/show_bug.cgi?id=689225 Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/47 ------------------------------------------------------------------------ On 2012-12-13T15:26:31+00:00 Stransky wrote: Created attachment 691815 WIP Adds FreeDesktop interface with SessionManager fallback. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/48 ------------------------------------------------------------------------ On 2012-12-18T12:40:11+00:00 Stransky wrote: Created attachment 693331 v2 Katl, what do you think about this one? The Inhibit() call is async. I use FreeDesktop interface with SessionManager fallback. Only the Fullscreen topic is handled. I'm not sure about the questions of ModifyWakeLock/Hal...shall we ask someone who works on that? Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/49 ------------------------------------------------------------------------ On 2013-01-08T04:29:07+00:00 Karlt wrote: Comment on attachment 693331 v2 How about disabling the screensaver only when "DOM_Fullscreen" and "Playing_media" are both in "locked-foreground" state? That would still lock the screen while a fullscreen window is playing audio, but at least that reduces the number of situations considerably. I don't want any DBus calls blocking on reply from the service, especially during nsAppShell::Init(), which would be called during start-up. I think it would be simplest, instead of testing different methods during initialization, to wait until after calling "Inhibit" on the freedesktop interface first. If an error is received from that call, then set some state to indicate that the freedesktop interface is not available and try again. When the freedesktop interface is not available, the gnome interface is tried. If that also fails, the state could even be updated to so that no interfaces are tried next time. These situations need to be handled: After a screenSaverInhibit() call, WakeLockListener::Callback() is called with a "locked-foreground" or other state before WakeLockListenerStatusNotify() receives the cookie for mInhibitRequest. >+LOCAL_INCLUDES += $(MOZ_DBUS_CFLAGS) >+CFLAGS += $(MOZ_DBUS_GLIB_CFLAGS) >+CXXFLAGS += $(MOZ_DBUS_GLIB_CFLAGS) -DHAVE_PTHREADS Do you know why -DHAVE_PTHREADS is here? I see one other instance of it beside MOZ_DBUS_GLIB_CFLAGS in Gecko, but other places don't have it. Please remove if it is not required. I expect MOZ_DBUS_CFLAGS is unnecessary with MOZ_DBUS_GLIB_CFLAGS. >+#include <dbus/dbus-glib.h> Looks like this may be unnecessary. >+ if(!topic.Equals(NS_LITERAL_STRING("DOM_Fullscreen"))) EqualsLiteral Remember Gecko style is "if (". >+nsCOMPtr<nsIPowerManagerService> sPowerManagerService = nullptr; >+nsCOMPtr<nsIDOMMozWakeLockListener> sWakeLockListener = nullptr; nsCOMPtr should not be used for global variables. (It runs constructors at startup.) I don't know whether or not StaticRefPtr would work here. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/50 ------------------------------------------------------------------------ On 2013-01-08T04:39:50+00:00 Karlt wrote: Comment on attachment 693331 v2 Justin, can you check that the use of nsIDOMMozWakeLockListener is as intended, and comment on whether the suggestion in comment 12 of listening for two topics is workable, please? I had trouble working out how the WakeLock code was meant to be used - see end of comment 3. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/51 ------------------------------------------------------------------------ On 2013-01-08T10:22:54+00:00 Justin-lebar+bug wrote: Comment on attachment 693331 v2 You got the wake lock API totally right except for one key point: Wake locks are not trusted. Any content can ask for a wake lock on anything. As currently designed, the fact that the "DOM_Fullscreen" wake lock is held tells you absolutely nothing; any webpage could hold that wake lock by doing navigator.requestWakeLock("DOM_Fullscreen"). I see that this mistake is all over the place. :( I'll try to get a handle on the bustage and file bugs. >+NS_IMPL_ISUPPORTS1(WakeLockListener, nsIDOMMozWakeLockListener) >+nsCOMPtr<nsIPowerManagerService> sPowerManagerService = nullptr; >+nsCOMPtr<nsIDOMMozWakeLockListener> sWakeLockListener = nullptr; A much more minor point: static nsCOMPtr/nsRefPtr/nsAutoPtr is an anti-pattern. It creates a static constructor, which slows startup. You can use |static StaticRefPtr|, but it's probably better to use members on nsAppShell. But more specifically, I don't think you need either of these refs. The power manager service will keep a strong ref to the wake lock listener, so you can just register the listener and be done with it. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/52 ------------------------------------------------------------------------ On 2013-01-08T11:12:12+00:00 Justin-lebar+bug wrote: I filed bug 827756 for the wake lock issues. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/53 ------------------------------------------------------------------------ On 2013-01-11T12:28:09+00:00 Stransky wrote: (In reply to Justin Lebar [:jlebar] from comment #14) > Comment on attachment 693331 > v2 > > You got the wake lock API totally right except for one key point: Wake locks > are not trusted. Any content can ask for a wake lock on anything. Justin, thanks for the clarification. So how we can get notification about the "screen" wake lock you're talking about in Bug 827756? I guess it should be driven by GeckoApp.java, right? Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/54 ------------------------------------------------------------------------ On 2013-01-14T07:28:12+00:00 Justin-lebar+bug wrote: > So how we can get notification about the "screen" wake lock you're talking > about in Bug > 827756? I'm not sure the screen wake lock is what you want here, though, since in its current form, any page can acquire the "screen" wake lock and keep the screensaver from appearing. I think you want a /trusted/ wake lock coming from Gecko, which is not a concept which currently exists in the API. I think I'd support adding something like that, if someone wanted to implement it. Anywho, to answer your question, if you just replace > + if(!topic.Equals(NS_LITERAL_STRING("DOM_Fullscreen"))) with "screen", you'll listen for the "screen" wake lock. At least, if I follow the code correctly (it's a bit tricky for me to follow since I'm not familiar with the dbus stuff). Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/55 ------------------------------------------------------------------------ On 2013-01-14T14:14:59+00:00 Stransky wrote: (In reply to Justin Lebar [:jlebar] from comment #17) > I think you want a /trusted/ wake lock coming from Gecko, which is not a > concept which currently exists in the API. I think I'd support adding > something like that, if someone wanted to implement it. Yes, it looks like something we need. I volunteer to implement it if you give me some hints how to design it. The untrusted locks from web pages is not a good idea. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/56 ------------------------------------------------------------------------ On 2013-01-14T15:37:04+00:00 Justin-lebar+bug wrote: > I volunteer to implement it if you give me some hints how to design it. Thanks! If you file a new bug I'll be happy to help out. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/57 ------------------------------------------------------------------------ On 2013-01-15T09:28:10+00:00 Stransky wrote: Filed as Bug 830660. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/58 ------------------------------------------------------------------------ On 2014-02-07T00:40:56+00:00 Cpearce-t wrote: (In reply to Justin Lebar (not reading bugmail) from comment #14) > Comment on attachment 693331 > v2 > > You got the wake lock API totally right except for one key point: Wake locks > are not trusted. Any content can ask for a wake lock on anything. Is this still the case? According to the documentation: https://developer.mozilla.org/en-US/docs/WebAPI/Power_Management Wake locks are only available to privileged apps. Indeed on desktop navigator.mozPower isn't visible, so I don't see why we should be worrying about trusted screen locks for desktop builds here. mounir: It was suggested you understand wake locks, do we still need the trusted wake lock service Martin implemented in bug 830660? If not I'd like to see the patch here landed ASAP. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/70 ------------------------------------------------------------------------ On 2014-02-07T00:54:01+00:00 Cpearce-t wrote: mounir no longer works for us, so may not be checking bugmail. kanru: you wrote the WakeLock, can you answer my question to mounir in comment 21 please? I'm trying to figure out if there's any reason why we can use "screen" wake lock listeners on desktop to disable the screen saver while video is playing. We're currently doing this in B2G, so I don't see why we shouldn't for Linux (and other desktop platforms) too. Justin Lebar previously said we needed a "trusted" wake lock to be implemented, but I don't understand why that should be necessary now, since mozPower is only accessible to content in privileged apps. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/71 ------------------------------------------------------------------------ On 2014-02-07T00:57:52+00:00 Cpearce-t wrote: Comment on attachment 693331 v2 Review of attachment 693331: ----------------------------------------------------------------- ::: widget/gtk2/nsAppShell.cpp @@ +167,5 @@ > + { > + if(!mConnection) > + return NS_ERROR_FAILURE; > + > + if(!topic.Equals(NS_LITERAL_STRING("DOM_Fullscreen"))) I don't think we should only disable the screen saver when we're playing fullscreen video. I think we should do it whenever video is playing, provided playback was initiated in a user generated event handler. I'll write a patch to ensure that we only send the notification if we're in a user generated event handler. In the meantime, we can make the topic here "screen". Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/72 ------------------------------------------------------------------------ On 2014-02-07T01:25:21+00:00 Cpearce-t wrote: Actually, I don't think that only disabling the screen saver in a user generated event handler would be a good experience. Some videos we'd want to disable the screen saver on start automatically. Like YouTube videos, they start playing automatically. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/73 ------------------------------------------------------------------------ On 2014-02-08T03:53:39+00:00 antistress wrote: Maybe disabling the screen could depend wether sounds are played or not ? Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/74 ------------------------------------------------------------------------ On 2014-02-09T02:51:31+00:00 Kchen-d wrote: (In reply to Chris Pearce (:cpearce) from comment #22) > mounir no longer works for us, so may not be checking bugmail. > > kanru: you wrote the WakeLock, can you answer my question to mounir in > comment 21 please? > > I'm trying to figure out if there's any reason why we can use "screen" wake > lock listeners on desktop to disable the screen saver while video is > playing. We're currently doing this in B2G, so I don't see why we shouldn't > for Linux (and other desktop platforms) too. > > Justin Lebar previously said we needed a "trusted" wake lock to be > implemented, but I don't understand why that should be necessary now, since > mozPower is only accessible to content in privileged apps. It's not about mozPower but navigator.requestWakeLock. Currently any web page could request a wake lock and keep screensaver from appearing. Note I'm going to land a patch for bug 963366 that hide wake locks from the web so using it in gecko is safe but this might change in the future. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/75 ------------------------------------------------------------------------ On 2014-02-09T21:39:55+00:00 Cpearce-t wrote: I think we should go ahead and try to get the wake lock listener here cleaned up and landed. Reply at: https://bugs.launchpad.net/ubuntu/+source/firefox-3.5/+bug/434476/comments/76 ** Changed in: firefox Status: Unknown => Confirmed ** Changed in: firefox Importance: Unknown => Wishlist ** Bug watch added: GNOME Bug Tracker #689225 https://bugzilla.gnome.org/show_bug.cgi?id=689225 -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to firefox in Ubuntu. https://bugs.launchpad.net/bugs/434476 Title: screensaver starts while playing HTML5 videos Status in The Mozilla Firefox Browser: Confirmed Status in “firefox” package in Ubuntu: New Status in “firefox-3.5” package in Ubuntu: Triaged Bug description: Binary package hint: firefox-3.5 karmic alpha6 + updates - click on a .ogv video link - after watching some time, screensaver starts. ProblemType: Bug Architecture: amd64 Date: Tue Sep 22 08:45:53 2009 DistroRelease: Ubuntu 9.10 NonfreeKernelModules: nvidia Package: firefox 3.5.3+build1+nobinonly-0ubuntu2 PackageArchitecture: all ProcEnviron: LANG=de_AT.UTF-8 SHELL=/bin/bash ProcVersionSignature: Ubuntu 2.6.31-10.34-generic SourcePackage: firefox-3.5 Uname: Linux 2.6.31-10-generic x86_64 To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/434476/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
