<div class="gmail_quote">On Wed, Jan 18, 2012 at 1:20 PM, Mirsal Ennaime <<a href="mailto:mirsal@videolan.org">mirsal@videolan.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Hello Kevin, and thank you for your patch, Comments inline. On Wed, 2012-01-18 at 12:47 -0500, Kevin Anthony wrote: > Here's the current patch, but i'm not sure what i should do when you said > "We also can't really say that the media player should implement these > additions, since we want to be backwards compatible" We can't require these new properties to be present without breaking backwards compatibility. > diff --git a/spec/Root_Node.xml b/spec/Root_Node.xml > index 191acaa..f96719e 100644 > --- a/spec/Root_Node.xml > +++ b/spec/Root_Node.xml > @@ -49,6 +49,41 @@ > </tp:docstring> > </property> > > + <property name="Fullscreen" type="b" > tp:name-for-bindings="Fullscreen" access="readwrite"> > + <tp:docstring xmlns="<a href="http://www.w3.org/1999/xhtml" target="_blank">http://www.w3.org/1999/xhtml</a>"> > + Causes the media player toggle into or out of fullscreen > mode. > + > + true toggles from normal mode to > fullscreen mode > + > + > + false toggles from fullscreen mode to > normal mode > + The term "toggle" might be a bit confusing, "switch" would probably be a better fit. > + > + The media player should ignore calls when > true and > + already in fullscreen mode or false when > not in fullscreen > + mode > + Using less ambiguous wording would be nice here, like: "The media player should ignore calls to org.freedesktop.DBus.Properties.Set for this property if the value given as an argument is the same as the current one" or something along these lines. > + > + The media player may refuse to allow clients to enter or > exit fullscreen. I would rather use "The media player may not allow..." but that's probably subjective. > + In this case, the > <tp:member-ref>CanSetFullscreen</tp:member-ref> property is > + false and this method does nothing. This is not a method, so "changing this property" instead of "this method" would be better IMHO, and we should state that CanSetFullscreen might not even exist. > + > + If the media player does niot have a UI, this should be > implemented there's a typo (s/niot/not), and we can't require these properties to be implemented. > + </tp:docstring> > + </property> > + > + <property name="CanSetFullscreen" type="b" > tp:name-for-bindings="Can_Set_Fullscreen" access="read"> > + <tp:docstring xmlns="<a href="http://www.w3.org/1999/xhtml" target="_blank">http://www.w3.org/1999/xhtml</a>"> > + > + If false, calling > + <tp:member-ref>Fullscreen</tp:member-ref> will have no > effect, and may Same as earlier, "changing the Fullscreen property" instead of "calling Fullscren" > + raise a NotSupported error. I don't think we should spec this: "and may raise an error" should be enough. Again, thank you very much for your time ! All the best, -- mirsal _______________________________________________ MPRIS mailing list <a href="mailto:MPRIS@lists.freedesktop.org">MPRIS@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mpris" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mpris</a> </blockquote></div> <div> </div>-- Thanks Kevin Anthony <a href="http://www.NoSideRacing.com" target="_blank">www.NoSideRacing.com</a><div> <div>Do you use Banshee?</div></div><div> Download the Community Extensions:</div><div><a href="http://banshee.fm/download/extensions/" target="_blank">http://banshee.fm/download/extensions/</a></div>