From ba9db4ebe4675cfa641c51f2c9a8897bc9bfe0cf Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Wed, 17 Dec 2008 13:27:46 +1300 Subject: [PATCH] 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 | 7 +++- layout/generic/nsVideoFrame.cpp | 40 ++++++++++++++++------- layout/generic/nsVideoFrame.h | 6 +++- layout/reftests/bugs/449149-1a.html | 6 ++++ layout/reftests/bugs/449149-1b.html | 10 ++++++ layout/reftests/bugs/449149-2-ref.html | 6 ++++ layout/reftests/bugs/449149-2.html | 6 ++++ layout/reftests/bugs/reftest.list | 3 ++ layout/style/forms.css | 6 ---- layout/style/html.css | 17 ++++++++++ toolkit/content/widgets/videocontrols.xml | 17 +++++++--- 11 files changed, 100 insertions(+), 24 deletions(-) create mode 100644 layout/reftests/bugs/449149-1a.html create mode 100644 layout/reftests/bugs/449149-1b.html create mode 100644 layout/reftests/bugs/449149-2-ref.html create mode 100644 layout/reftests/bugs/449149-2.html diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 848f7980070..a3936095dde 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/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; diff --git a/layout/generic/nsVideoFrame.cpp b/layout/generic/nsVideoFrame.cpp index 223e070d889..0024d52c2e3 100644 --- a/layout/generic/nsVideoFrame.cpp +++ b/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(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 videoElem = do_QueryInterface(mContent); + return videoElem != nsnull; +} diff --git a/layout/generic/nsVideoFrame.h b/layout/generic/nsVideoFrame.h index 1f4882ad583..53a29b7b080 100644 --- a/layout/generic/nsVideoFrame.h +++ b/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; diff --git a/layout/reftests/bugs/449149-1a.html b/layout/reftests/bugs/449149-1a.html new file mode 100644 index 00000000000..ad6343c44f4 --- /dev/null +++ b/layout/reftests/bugs/449149-1a.html @@ -0,0 +1,6 @@ + + + + + + diff --git a/layout/reftests/bugs/449149-1b.html b/layout/reftests/bugs/449149-1b.html new file mode 100644 index 00000000000..61cf890af48 --- /dev/null +++ b/layout/reftests/bugs/449149-1b.html @@ -0,0 +1,10 @@ + + + + + + + diff --git a/layout/reftests/bugs/449149-2-ref.html b/layout/reftests/bugs/449149-2-ref.html new file mode 100644 index 00000000000..e6a406551fc --- /dev/null +++ b/layout/reftests/bugs/449149-2-ref.html @@ -0,0 +1,6 @@ + + + +
+ + diff --git a/layout/reftests/bugs/449149-2.html b/layout/reftests/bugs/449149-2.html new file mode 100644 index 00000000000..25dab242bd9 --- /dev/null +++ b/layout/reftests/bugs/449149-2.html @@ -0,0 +1,6 @@ + + + + + + diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index a0eb928528e..39f32d0c024 100644 --- a/layout/reftests/bugs/reftest.list +++ b/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 diff --git a/layout/style/forms.css b/layout/style/forms.css index d935d07f472..36d23149356 100644 --- a/layout/style/forms.css +++ b/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 diff --git a/layout/style/html.css b/layout/style/html.css index dcd21412a3c..fd23037fcbf 100644 --- a/layout/style/html.css +++ b/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 tag */ marquee { width: -moz-available; diff --git a/toolkit/content/widgets/videocontrols.xml b/toolkit/content/widgets/videocontrols.xml index e5f44ca17bb..31089310044 100644 --- a/toolkit/content/widgets/videocontrols.xml +++ b/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 @@