Bug 449149. Implement the 'controls' attribute for audio elements. r+sr=bzbarsky,r=dolske,r=enndeakin

--HG--
extra : rebase_source : 4d11b963d3082f98269069c68aae33eef365aacb

layout/base/nsCSSFrameConstructor.cpp

@@ -3306,6 +3306,7 @@ IsSpecialContent(nsIContent*     aContent,
       aTag == nsGkAtoms::canvas ||
 #if defined(MOZ_MEDIA)
       aTag == nsGkAtoms::video ||
+      aTag == nsGkAtoms::audio ||
 #endif
       PR_FALSE;
   }
@@ -5549,7 +5550,10 @@ nsCSSFrameConstructor::ConstructHTMLFrame(nsFrameConstructorState& aState,
     triedFrame = PR_TRUE;
   }
 #if defined(MOZ_MEDIA)
-  else if (nsGkAtoms::video == aTag) {
+  else if (nsGkAtoms::video == aTag || nsGkAtoms::audio == aTag) {
+    // We create video frames for audio elements so we can show controls.
+    // Note that html.css specifies display:none for audio elements
+    // without the "controls" attribute.
     if (!aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aFrameItems); 
     }
@@ -5672,6 +5676,7 @@ nsCSSFrameConstructor::CreateAnonymousFrames(nsIAtom*                 aTag,
 #endif
 #ifdef MOZ_MEDIA
       && aTag != nsGkAtoms::video
+      && aTag != nsGkAtoms::audio
 #endif
       )
     return NS_OK;

layout/generic/nsVideoFrame.cpp

@@ -221,8 +221,10 @@ nsVideoFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   nsresult rv = DisplayBorderBackgroundOutline(aBuilder, aLists);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aLists.Content()->AppendNewToTop(new (aBuilder) nsDisplayGeneric(this, ::PaintVideo, "Video"));
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (HasVideoData()) {
+    rv = aLists.Content()->AppendNewToTop(new (aBuilder) nsDisplayGeneric(this, ::PaintVideo, "Video"));
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
 
   if (mFrames.FirstChild()) {
     rv = mFrames.FirstChild()->BuildDisplayListForStackingContext(aBuilder, aDirtyRect, aLists.Content());
@@ -260,7 +262,7 @@ nsSize nsVideoFrame::ComputeSize(nsIRenderingContext *aRenderingContext,
                                      nsSize aPadding,
                                      PRBool aShrinkWrap)
 {
-  nsSize size = GetVideoSize();
+  nsSize size = GetIntrinsicSize(aRenderingContext);
 
   IntrinsicSize intrinsicSize;
   intrinsicSize.width.SetCoordValue(size.width);
@@ -280,32 +282,42 @@ nsSize nsVideoFrame::ComputeSize(nsIRenderingContext *aRenderingContext,
 
 nscoord nsVideoFrame::GetMinWidth(nsIRenderingContext *aRenderingContext)
 {
-  // XXX The caller doesn't account for constraints of the height,
-  // min-height, and max-height properties.
-  nscoord result = GetVideoSize().width;
+  nscoord result = GetIntrinsicSize(aRenderingContext).width;
   DISPLAY_MIN_WIDTH(this, result);
   return result;
 }
 
 nscoord nsVideoFrame::GetPrefWidth(nsIRenderingContext *aRenderingContext)
 {
-  // XXX The caller doesn't account for constraints of the height,
-  // min-height, and max-height properties.
-  nscoord result = GetVideoSize().width;
+  nscoord result = GetIntrinsicSize(aRenderingContext).width;
   DISPLAY_PREF_WIDTH(this, result);
   return result;
 }
 
 nsSize nsVideoFrame::GetIntrinsicRatio()
 {
-  return GetVideoSize();
+  return GetIntrinsicSize(nsnull);
 }
 
-nsSize nsVideoFrame::GetVideoSize()
+nsSize nsVideoFrame::GetIntrinsicSize(nsIRenderingContext *aRenderingContext)
 {
   // Defaulting size to 300x150 if no size given.
   nsIntSize size(300,150);
 
+  if (!HasVideoData()) {
+    if (!aRenderingContext || !mFrames.FirstChild()) {
+      // We just want our intrinsic ratio, but audio elements need no
+      // intrinsic ratio, so just return "no ratio". Also, if there's
+      // no controls frame, we prefer to be zero-sized.
+      return nsSize(0, 0);
+    }
+
+    // Ask the controls frame what its preferred height is
+    nsBoxLayoutState boxState(PresContext(), aRenderingContext, 0);
+    nscoord prefHeight = mFrames.FirstChild()->GetPrefSize(boxState).height;
+    return nsSize(nsPresContext::CSSPixelsToAppUnits(size.width), prefHeight);
+  }
+
   nsHTMLVideoElement* element = static_cast<nsHTMLVideoElement*>(GetContent());
   if (element) {
     nsresult rv;
@@ -330,3 +342,9 @@ nsSize nsVideoFrame::GetVideoSize()
   return nsSize(nsPresContext::CSSPixelsToAppUnits(size.width), 
                 nsPresContext::CSSPixelsToAppUnits(size.height));
 }
+
+PRBool nsVideoFrame::HasVideoData()
+{
+  nsCOMPtr<nsIDOMHTMLVideoElement> videoElem = do_QueryInterface(mContent);
+  return videoElem != nsnull;
+}

layout/generic/nsVideoFrame.h

@@ -67,7 +67,7 @@ public:
                    const nsRect& aDirtyRect, nsPoint aPt);
                               
   /* get the size of the video's display */
-  nsSize GetVideoSize();
+  nsSize GetIntrinsicSize(nsIRenderingContext *aRenderingContext);
   virtual nsSize GetIntrinsicRatio();
   virtual nsSize ComputeSize(nsIRenderingContext *aRenderingContext,
                              nsSize aCBSize, nscoord aAvailableWidth,
@@ -101,6 +101,10 @@ public:
 #endif
 
 protected:
+  // Returns true if there is video data to render. Can return false
+  // when we're the frame for an audio element.
+  PRBool HasVideoData();
+
   virtual ~nsVideoFrame();
 
   nsMargin mBorderPadding;

layout/reftests/bugs/449149-1a.html

@@ -0,0 +1,6 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<audio controls></audio>
+</body>
+</html>

layout/reftests/bugs/449149-1b.html

@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<audio id="a"></audio>
+<script>
+document.body.offsetTop;
+document.getElementById("a").setAttribute("controls", "");
+</script>
+</body>
+</html>

layout/reftests/bugs/449149-2-ref.html

@@ -0,0 +1,6 @@
+<!DOCTYPE HTML>
+<html>
+<body style="background:yellow;">
+<div style="display:block; width:100px; height:100px; border:2px solid blue;"></div>
+</body>
+</html>

layout/reftests/bugs/449149-2.html

@@ -0,0 +1,6 @@
+<!DOCTYPE HTML>
+<html>
+<body style="background:yellow;">
+<audio style="display:block; width:100px; height:100px; border:2px solid blue;"></audio>
+</body>
+</html>

layout/reftests/bugs/reftest.list

@@ -943,6 +943,9 @@ fails == 441259-2.html 441259-2-ref.html # bug 441400
 == 446100-1h.html about:blank
 # == 448193.html 448193-ref.html  # Fails due to 2 small single-pixel differences
 # == 448987.html 448987-ref.html  # Disabled for now - it needs privileges
+!= 449149-1a.html about:blank
+!= 449149-1b.html about:blank
+== 449149-2.html 449149-2-ref.html
 == 449171-1.html 449171-ref.html
 == 449519-1.html 449519-1-ref.html
 # == 449653-1.html 449653-1-ref.html # Disabled for now - it needs privileges

layout/style/forms.css

@@ -599,12 +599,6 @@ input[type="file"] > input[type="text"] {
   input[type="file"] { height: 2em; }
 }
 
-video > xul|videocontrols {
-  display: -moz-box;
-  -moz-box-orient: vertical;
-  -moz-binding: url("chrome://global/content/bindings/videocontrols.xml#videoControls");
-}
-
 %if OSARCH==OS2
 input {
   font: medium serif; font-family: inherit

layout/style/html.css

@@ -36,6 +36,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 @namespace url(http://www.w3.org/1999/xhtml); /* set default namespace to HTML */
+@namespace xul url(http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul);
 
 /* bidi */
 
@@ -487,6 +488,22 @@ noembed, param {
    display: none;
 }
 
+/* media elements */
+video > xul|videocontrols, audio > xul|videocontrols {
+  display: -moz-box;
+  -moz-box-orient: vertical;
+  -moz-binding: url("chrome://global/content/bindings/videocontrols.xml#videoControls");
+}
+
+video:not([controls]) > xul|videocontrols,
+audio:not([controls]) > xul|videocontrols {
+  visibility: hidden;
+}
+
+audio:not([controls]) {
+  display: none;
+}
+
 /* emulation of non-standard HTML <marquee> tag */
 marquee {
   width: -moz-available;

toolkit/content/widgets/videocontrols.xml

@@ -85,6 +85,7 @@
                 animationStep : 0,
                 animationDirection : 1,
                 animationTimer : null,
+                dynamicHideControls : true,
 
                 handleEvent : function (aEvent) {
                     this.log("Got " + aEvent.type + " media event");
@@ -163,6 +164,8 @@
             var video = this.parentNode;
             this.Utils.video = video;
             this.Utils.controls = this;
+            // We shouldn't dynamically hide the controls for audio elements
+            this.Utils.dynamicHideControls = video instanceof HTMLVideoElement;
 
             this.Utils.playButton = document.getAnonymousElementByAttribute(this, "id", "playButton");
             this.Utils.muteButton = document.getAnonymousElementByAttribute(this, "id", "muteButton");
@@ -170,7 +173,9 @@
             // Set initial state of play/pause button.
             this.Utils.playButton.setAttribute("paused", video.paused);
 
-            this.style.opacity = 0;
+            if (this.Utils.dynamicHideControls) {
+                this.style.opacity = 0;
+            }
 
             // Use Utils.handleEvent() callback for all media events.
             video.addEventListener("play",         this.Utils, false);
@@ -188,15 +193,14 @@
     <handlers>
         <handler event="mouseover">
             <![CDATA[
+            if (!this.Utils.dynamicHideControls)
+                return;
+
             // Ignore events caused by transitions between child nodes.
             if (this.Utils.isChildNode(event.relatedTarget))
                 return;
 
-            // Don't show controls unless they're enabled
-            // XXX sucks that spec defaults to controls=false. :-(
             // XXX add support for dynamically hiding controls w/o waiting for mouseout?
-            if (!this.Utils.video.controls)
-                return;
 
             this.Utils.animationDirection = 1;
             if (!this.Utils.animationTimer)
@@ -206,6 +210,9 @@
 
         <handler event="mouseout">
             <![CDATA[
+            if (!this.Utils.dynamicHideControls)
+                return;
+
             // Ignore events caused by transitions between child nodes.
             if (this.Utils.isChildNode(event.relatedTarget))
                 return;