- Revision
- 170920
- Author
- eric.carl...@apple.com
- Date
- 2014-07-09 10:17:16 -0700 (Wed, 09 Jul 2014)
Log Message
[iOS] caption size is sometimes incorrect in fullscreen
https://bugs.webkit.org/show_bug.cgi?id=134740
Reviewed by Jer Noble.
Captions on iOS are displayed in fullscreen with a TextTrackRepresentation object. Because
the fullscreen video presentation is controlled by code in the UI process running on the
UI thread, WebCore is notified of changes to fullscreen state asynchronously. This resulted
in the TextTrackRepresentation object being created and/or destroyed too late some of the
time, which caused us to sometimes display captions incorrectly. Fix this by setting up and
tearing down the TextTrackRepresentation object when WebCore's 'webkitfullscreenchange'
event fires.
* Modules/mediacontrols/MediaControlsHost.cpp:
(WebCore::MediaControlsHost::enteredFullscreen): Notify text track container.
(WebCore::MediaControlsHost::exitedFullscreen): Ditto.
* Modules/mediacontrols/MediaControlsHost.h:
* Modules/mediacontrols/MediaControlsHost.idl:
* Modules/mediacontrols/mediaControlsApple.js:
(Controller.prototype.handleFullscreenChange): Notify host of fullscreen change.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::requiresTextTrackRepresentation): Only return true when in
fullscreen.
(WebCore::HTMLMediaElement::setVideoFullscreenLayer): Call updateTextTrackDisplay.
* html/shadow/MediaControlElements.cpp:
(WebCore::MediaControlTextTrackContainerElement::MediaControlTextTrackContainerElement): Initialize
m_updateTextTrackRepresentationStyle to false.
(WebCore::MediaControlTextTrackContainerElement::updateDisplay): Move logic for creating
TextTrackRepresentation to updateTextTrackRepresentation.
(WebCore::MediaControlTextTrackContainerElement::updateActiveCuesFontSize): New, split out of updateTimerFired.
(WebCore::MediaControlTextTrackContainerElement::updateTimerFired): Move code to force immediate
font size change to updateActiveCuesFontSize.
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentation): New, update
text track representation, creating first if necessary.
(WebCore::MediaControlTextTrackContainerElement::clearTextTrackRepresentation): Add an early
return if we don't have a text track representation.
(WebCore::MediaControlTextTrackContainerElement::updateStyleForTextTrackRepresentation): Early
return if there is nothing to be done.
(WebCore::MediaControlTextTrackContainerElement::enteredFullscreen): Force a caption update
if there are visible captions.
(WebCore::MediaControlTextTrackContainerElement::updateSizes): Set m_updateTextTrackRepresentationStyle
to true.
(WebCore::MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged): Force a
caption update if there are visible captions.
* html/shadow/MediaControlElements.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (170919 => 170920)
--- trunk/Source/WebCore/ChangeLog 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/ChangeLog 2014-07-09 17:17:16 UTC (rev 170920)
@@ -1,3 +1,54 @@
+2014-07-09 Eric Carlson <eric.carl...@apple.com>
+
+ [iOS] caption size is sometimes incorrect in fullscreen
+ https://bugs.webkit.org/show_bug.cgi?id=134740
+
+ Reviewed by Jer Noble.
+
+ Captions on iOS are displayed in fullscreen with a TextTrackRepresentation object. Because
+ the fullscreen video presentation is controlled by code in the UI process running on the
+ UI thread, WebCore is notified of changes to fullscreen state asynchronously. This resulted
+ in the TextTrackRepresentation object being created and/or destroyed too late some of the
+ time, which caused us to sometimes display captions incorrectly. Fix this by setting up and
+ tearing down the TextTrackRepresentation object when WebCore's 'webkitfullscreenchange'
+ event fires.
+
+ * Modules/mediacontrols/MediaControlsHost.cpp:
+ (WebCore::MediaControlsHost::enteredFullscreen): Notify text track container.
+ (WebCore::MediaControlsHost::exitedFullscreen): Ditto.
+ * Modules/mediacontrols/MediaControlsHost.h:
+ * Modules/mediacontrols/MediaControlsHost.idl:
+
+ * Modules/mediacontrols/mediaControlsApple.js:
+ (Controller.prototype.handleFullscreenChange): Notify host of fullscreen change.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::requiresTextTrackRepresentation): Only return true when in
+ fullscreen.
+ (WebCore::HTMLMediaElement::setVideoFullscreenLayer): Call updateTextTrackDisplay.
+
+ * html/shadow/MediaControlElements.cpp:
+ (WebCore::MediaControlTextTrackContainerElement::MediaControlTextTrackContainerElement): Initialize
+ m_updateTextTrackRepresentationStyle to false.
+ (WebCore::MediaControlTextTrackContainerElement::updateDisplay): Move logic for creating
+ TextTrackRepresentation to updateTextTrackRepresentation.
+ (WebCore::MediaControlTextTrackContainerElement::updateActiveCuesFontSize): New, split out of updateTimerFired.
+ (WebCore::MediaControlTextTrackContainerElement::updateTimerFired): Move code to force immediate
+ font size change to updateActiveCuesFontSize.
+ (WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentation): New, update
+ text track representation, creating first if necessary.
+ (WebCore::MediaControlTextTrackContainerElement::clearTextTrackRepresentation): Add an early
+ return if we don't have a text track representation.
+ (WebCore::MediaControlTextTrackContainerElement::updateStyleForTextTrackRepresentation): Early
+ return if there is nothing to be done.
+ (WebCore::MediaControlTextTrackContainerElement::enteredFullscreen): Force a caption update
+ if there are visible captions.
+ (WebCore::MediaControlTextTrackContainerElement::updateSizes): Set m_updateTextTrackRepresentationStyle
+ to true.
+ (WebCore::MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged): Force a
+ caption update if there are visible captions.
+ * html/shadow/MediaControlElements.h:
+
2014-07-09 Jer Noble <jer.no...@apple.com>
[MSE] http/tests/media/media-source/mediasource-endofstream-invaliderror.html is failing.
Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp (170919 => 170920)
--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp 2014-07-09 17:17:16 UTC (rev 170920)
@@ -150,6 +150,18 @@
m_textTrackContainer->updateDisplay();
}
+void MediaControlsHost::enteredFullscreen()
+{
+ if (m_textTrackContainer)
+ m_textTrackContainer->enteredFullscreen();
+}
+
+void MediaControlsHost::exitedFullscreen()
+{
+ if (m_textTrackContainer)
+ m_textTrackContainer->exitedFullscreen();
+}
+
void MediaControlsHost::updateCaptionDisplaySizes()
{
if (m_textTrackContainer)
Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h (170919 => 170920)
--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h 2014-07-09 17:17:16 UTC (rev 170920)
@@ -64,6 +64,8 @@
bool userGestureRequired() const;
void updateCaptionDisplaySizes();
+ void enteredFullscreen();
+ void exitedFullscreen();
String externalDeviceDisplayName() const;
String externalDeviceType() const;
Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl (170919 => 170920)
--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl 2014-07-09 17:17:16 UTC (rev 170920)
@@ -41,7 +41,6 @@
readonly attribute DOMString captionDisplayMode;
void setSelectedTextTrack(TextTrack track);
readonly attribute HTMLElement textTrackContainer;
- void updateTextTrackContainer();
readonly attribute boolean mediaPlaybackAllowsInline;
readonly attribute boolean supportsFullscreen;
readonly attribute boolean userGestureRequired;
@@ -50,4 +49,8 @@
readonly attribute DeviceType externalDeviceType;
attribute boolean controlsDependOnPageScaleFactor;
+
+ void updateTextTrackContainer();
+ void enteredFullscreen();
+ void exitedFullscreen();
};
Modified: trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js (170919 => 170920)
--- trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js 2014-07-09 17:17:16 UTC (rev 170920)
@@ -624,9 +624,11 @@
if (this.isFullScreen()) {
this.controls.fullscreenButton.classList.add(this.ClassNames.exit);
this.controls.fullscreenButton.setAttribute('aria-label', this.UIString('Exit Full Screen'));
+ this.host.enteredFullscreen();
} else {
this.controls.fullscreenButton.classList.remove(this.ClassNames.exit);
this.controls.fullscreenButton.setAttribute('aria-label', this.UIString('Display Full Screen'));
+ this.host.exitedFullscreen();
}
},
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (170919 => 170920)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-07-09 17:17:16 UTC (rev 170920)
@@ -4773,7 +4773,7 @@
#if ENABLE(VIDEO_TRACK)
bool HTMLMediaElement::requiresTextTrackRepresentation() const
{
- return m_player ? m_player->requiresTextTrackRepresentation() : 0;
+ return m_isFullscreen && m_player ? m_player->requiresTextTrackRepresentation() : false;
}
void HTMLMediaElement::setTextTrackRepresentation(TextTrackRepresentation* representation)
@@ -4961,6 +4961,10 @@
m_player->setVideoFullscreenLayer(platformLayer);
setNeedsStyleRecalc(SyntheticStyleChange);
+#if ENABLE(VIDEO_TRACK)
+ if (RuntimeEnabledFeatures::sharedFeatures().webkitVideoTrackEnabled())
+ updateTextTrackDisplay();
+#endif
}
void HTMLMediaElement::setVideoFullscreenFrame(FloatRect frame)
Modified: trunk/Source/WebCore/html/shadow/MediaControlElements.cpp (170919 => 170920)
--- trunk/Source/WebCore/html/shadow/MediaControlElements.cpp 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/html/shadow/MediaControlElements.cpp 2014-07-09 17:17:16 UTC (rev 170920)
@@ -1228,6 +1228,7 @@
, m_updateTimer(this, &MediaControlTextTrackContainerElement::updateTimerFired)
, m_fontSize(0)
, m_fontSizeIsImportant(false)
+ , m_updateTextTrackRepresentationStyle(false)
{
setPseudo(shadowPseudoId());
}
@@ -1353,28 +1354,18 @@
// 11. Return output.
if (hasChildNodes()) {
show();
- if (mediaElement->requiresTextTrackRepresentation()) {
- if (!m_textTrackRepresentation)
- m_textTrackRepresentation = TextTrackRepresentation::create(this);
- mediaElement->setTextTrackRepresentation(m_textTrackRepresentation.get());
-
- m_textTrackRepresentation->update();
- updateStyleForTextTrackRepresentation();
- }
+ updateTextTrackRepresentation();
} else {
hide();
clearTextTrackRepresentation();
}
}
-void MediaControlTextTrackContainerElement::updateTimerFired(Timer<MediaControlTextTrackContainerElement>&)
+void MediaControlTextTrackContainerElement::updateActiveCuesFontSize()
{
if (!document().page())
return;
- if (m_textTrackRepresentation)
- updateStyleForTextTrackRepresentation();
-
HTMLMediaElement* mediaElement = parentMediaElement(this);
if (!mediaElement)
return;
@@ -1391,36 +1382,79 @@
toVTTCue(cue)->setFontSize(m_fontSize, m_videoDisplaySize.size(), m_fontSizeIsImportant);
}
+
+}
+
+void MediaControlTextTrackContainerElement::updateTimerFired(Timer<MediaControlTextTrackContainerElement>&)
+{
+ if (!document().page())
+ return;
+
+ if (m_textTrackRepresentation)
+ updateStyleForTextTrackRepresentation();
+
+ updateActiveCuesFontSize();
updateDisplay();
}
+void MediaControlTextTrackContainerElement::updateTextTrackRepresentation()
+{
+ HTMLMediaElement* mediaElement = parentMediaElement(this);
+ if (!mediaElement)
+ return;
+
+ if (!mediaElement->requiresTextTrackRepresentation())
+ return;
+
+ if (!m_textTrackRepresentation) {
+ m_textTrackRepresentation = TextTrackRepresentation::create(this);
+ m_updateTextTrackRepresentationStyle = true;
+ mediaElement->setTextTrackRepresentation(m_textTrackRepresentation.get());
+ }
+
+ m_textTrackRepresentation->update();
+ updateStyleForTextTrackRepresentation();
+}
+
void MediaControlTextTrackContainerElement::clearTextTrackRepresentation()
{
+ if (!m_textTrackRepresentation)
+ return;
+
+ m_textTrackRepresentation = nullptr;
+ m_updateTextTrackRepresentationStyle = true;
if (HTMLMediaElement* mediaElement = parentMediaElement(this))
mediaElement->setTextTrackRepresentation(nullptr);
- m_textTrackRepresentation = nullptr;
updateStyleForTextTrackRepresentation();
+ updateActiveCuesFontSize();
}
void MediaControlTextTrackContainerElement::updateStyleForTextTrackRepresentation()
{
+ if (!m_updateTextTrackRepresentationStyle)
+ return;
+ m_updateTextTrackRepresentationStyle = false;
+
if (m_textTrackRepresentation) {
setInlineStyleProperty(CSSPropertyWidth, m_videoDisplaySize.size().width(), CSSPrimitiveValue::CSS_PX);
setInlineStyleProperty(CSSPropertyHeight, m_videoDisplaySize.size().height(), CSSPrimitiveValue::CSS_PX);
setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
setInlineStyleProperty(CSSPropertyLeft, 0, CSSPrimitiveValue::CSS_PX);
setInlineStyleProperty(CSSPropertyTop, 0, CSSPrimitiveValue::CSS_PX);
- } else {
- removeInlineStyleProperty(CSSPropertyPosition);
- removeInlineStyleProperty(CSSPropertyWidth);
- removeInlineStyleProperty(CSSPropertyHeight);
- removeInlineStyleProperty(CSSPropertyLeft);
- removeInlineStyleProperty(CSSPropertyTop);
+ return;
}
+
+ removeInlineStyleProperty(CSSPropertyPosition);
+ removeInlineStyleProperty(CSSPropertyWidth);
+ removeInlineStyleProperty(CSSPropertyHeight);
+ removeInlineStyleProperty(CSSPropertyLeft);
+ removeInlineStyleProperty(CSSPropertyTop);
}
void MediaControlTextTrackContainerElement::enteredFullscreen()
{
+ if (hasChildNodes())
+ updateTextTrackRepresentation();
updateSizes(true);
}
@@ -1442,7 +1476,6 @@
mediaElement->syncTextTrackBounds();
IntRect videoBox;
-
if (m_textTrackRepresentation)
videoBox = m_textTrackRepresentation->bounds();
else {
@@ -1453,7 +1486,9 @@
if (!forceUpdate && m_videoDisplaySize == videoBox)
return;
+
m_videoDisplaySize = videoBox;
+ m_updateTextTrackRepresentationStyle = true;
// FIXME (121170): This function is called during layout, and should lay out the text tracks immediately.
m_updateTimer.startOneShot(0);
@@ -1496,6 +1531,8 @@
void MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged(const IntRect&)
{
+ if (hasChildNodes())
+ updateTextTrackRepresentation();
updateSizes();
}
Modified: trunk/Source/WebCore/html/shadow/MediaControlElements.h (170919 => 170920)
--- trunk/Source/WebCore/html/shadow/MediaControlElements.h 2014-07-09 17:12:39 UTC (rev 170919)
+++ trunk/Source/WebCore/html/shadow/MediaControlElements.h 2014-07-09 17:17:16 UTC (rev 170920)
@@ -478,6 +478,7 @@
private:
void updateTimerFired(Timer<MediaControlTextTrackContainerElement>&);
+ void updateActiveCuesFontSize();
explicit MediaControlTextTrackContainerElement(Document&);
virtual const AtomicString& shadowPseudoId() const override;
@@ -486,6 +487,7 @@
virtual PassRefPtr<Image> createTextTrackRepresentationImage() override;
virtual void textTrackRepresentationBoundsChanged(const IntRect&) override;
+ void updateTextTrackRepresentation();
void clearTextTrackRepresentation();
void updateStyleForTextTrackRepresentation();
OwnPtr<TextTrackRepresentation> m_textTrackRepresentation;
@@ -494,6 +496,7 @@
IntRect m_videoDisplaySize;
int m_fontSize;
bool m_fontSizeIsImportant;
+ bool m_updateTextTrackRepresentationStyle;
};
#endif
