Cherry-pick 53b6a4522e45. rdar://119138261
REGRESSION(265596@main): [Cocoa] In-band captions generate large, line-wrapped cues
https://bugs.webkit.org/show_bug.cgi?id=265635
rdar://119024855
Reviewed by Eric Carlson.
When modernizing VTTCue, TextTrackCueGeneric was not sufficiently updated to take advantage
of the correct layout implemented by VTTCue. Specifically, TextTrackCueGeneric still queried
VTTCueBox::fontSizeFromCaptionUserPrefs(), which was no longer set, and caused the cue width
to be incorrectly set to zero.
Remove that dead method, and remove most of the implementation of
TextTrackCueGenericBoxElement::applyCSSProperties, relying instead upon VTTCueBoxElement's
implementation.
In order to correctly position TextTrackCueGeneric objects created from AVFoundation, set
the resulting cue's positionAlign() to "Center" by way of GenericCueData. This causes VTTCue
to position the cue relative to the cue's center point, which is the same way AVFoundation
represents the cue's position.
* LayoutTests/media/content/test-inband-captions.mp4: Added.
* LayoutTests/media/track/track-in-band-layout-expected.txt: Added.
* LayoutTests/media/track/track-in-band-layout.html: Added.
* Source/WebCore/html/track/InbandGenericTextTrack.cpp:
(WebCore::InbandGenericTextTrack::updateCueFromCueData):
* Source/WebCore/html/track/TextTrackCueGeneric.cpp:
(WebCore::TextTrackCueGenericBoxElement::applyCSSProperties):
(WebCore::TextTrackCueGeneric::setBaseFontSizeRelativeToVideoHeight):
(WebCore::TextTrackCueGeneric::setFontSizeMultiplier):
(WebCore::TextTrackCueGeneric::setFontSize): Deleted.
* Source/WebCore/html/track/TextTrackCueGeneric.h:
* Source/WebCore/html/track/VTTCue.h:
(WebCore::VTTCueBox::setFontSizeFromCaptionUserPrefs): Deleted.
(WebCore::VTTCueBox::fontSizeFromCaptionUserPrefs const): Deleted.
* Source/WebCore/platform/graphics/InbandGenericCue.cpp:
(WebCore::InbandGenericCue::toJSONString const):
* Source/WebCore/platform/graphics/InbandGenericCue.h:
(WebCore::GenericCueData::GenericCueData):
(WebCore::InbandGenericCue::positionAlign const):
(WebCore::InbandGenericCue::setPositionAlign):
* Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:
(WebCore::InbandTextTrackPrivateAVF::processAttributedStrings):
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
Canonical link: https://commits.webkit.org/271415@main
Identifier: 267815.602@safari-7617.2.1.11-branch
diff --git a/LayoutTests/media/content/test-inband-captions.mp4 b/LayoutTests/media/content/test-inband-captions.mp4
new file mode 100644
index 0000000..aa691d9
--- /dev/null
+++ b/LayoutTests/media/content/test-inband-captions.mp4
Binary files differ
diff --git a/LayoutTests/media/track/track-in-band-layout-expected.txt b/LayoutTests/media/track/track-in-band-layout-expected.txt
new file mode 100644
index 0000000..b41ac62
--- /dev/null
+++ b/LayoutTests/media/track/track-in-band-layout-expected.txt
@@ -0,0 +1,17 @@
+
+RUN(video.src = "../content/test-inband-captions.mp4")
+EVENT(addtrack)
+RUN(video.textTracks[0].mode = "showing")
+RUN(video.play())
+EXPECTED (video.textTracks[0].cues.length > '0') OK
+RUN(video.pause())
+RUN(video.currentTime = video.textTracks[0].cues[0].startTime + 0.1)
+EVENT(seeked)
+EXPECTED (video.textTracks[0].activeCues.length == '1') OK
+EXPECTED (video.textTracks[0].activeCues[0].text == 'Align Top Left') OK
+
+Ensure the cue does not wrap:
+EXPECTED (firstCueElement().getClientRects().length == '1') OK
+EXPECTED (firstCueElement().getClientRects()[0].height < '22') OK
+END OF TEST
+
diff --git a/LayoutTests/media/track/track-in-band-layout.html b/LayoutTests/media/track/track-in-band-layout.html
new file mode 100644
index 0000000..984db3b
--- /dev/null
+++ b/LayoutTests/media/track/track-in-band-layout.html
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+ <title>track-in-band-layout</title>
+ <script src="../video-test.js"></script>
+ <script>
+ function firstCueElement() {
+ return internals.shadowRoot(video).querySelector('[pseudo="cue"]');
+ }
+
+ async function runTest() {
+ findMediaElement();
+
+ run('video.src = "../content/test-inband-captions.mp4"');
+ await waitFor(video.textTracks, 'addtrack');
+ run('video.textTracks[0].mode = "showing"');
+
+ run('video.play()');
+ await testExpectedEventually('video.textTracks[0].cues.length', 0, '>');
+
+ run('video.pause()');
+
+ run('video.currentTime = video.textTracks[0].cues[0].startTime + 0.1');
+ await waitFor(video, 'seeked');
+
+ testExpected('video.textTracks[0].activeCues.length', 1);
+ testExpected('video.textTracks[0].activeCues[0].text', 'Align Top Left');
+
+ consoleWrite('')
+ consoleWrite('Ensure the cue does not wrap:');
+ await testExpectedEventually('firstCueElement().getClientRects().length', 1);
+ await testExpectedEventually('firstCueElement().getClientRects()[0].height', 22, '<');
+ }
+
+ window.addEventListener('load', event => {
+ runTest().then(endTest).catch(failTest);
+ })
+ </script>
+</head>
+<body>
+ <video controls muted preload="auto">
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
index 66495d1..9f46421 100644
--- a/LayoutTests/platform/glib/TestExpectations
+++ b/LayoutTests/platform/glib/TestExpectations
@@ -2386,6 +2386,7 @@
# No support for MPEG-4 caption support
webkit.org/b/131546 media/track/track-forced-subtitles-in-band.html [ Timeout Failure Crash ]
media/track/track-in-band-chapters.html [ Timeout Failure ]
+media/track/track-in-band-layout.html [ Skip ]
# ENABLE(WEBVTT_REGIONS) is disabled
webkit.org/b/109570 media/track/regions-webvtt [ Skip ]
diff --git a/Source/WebCore/html/track/InbandGenericTextTrack.cpp b/Source/WebCore/html/track/InbandGenericTextTrack.cpp
index 30a7223..c37f1cd 100644
--- a/Source/WebCore/html/track/InbandGenericTextTrack.cpp
+++ b/Source/WebCore/html/track/InbandGenericTextTrack.cpp
@@ -107,6 +107,20 @@
if (inbandCue.highlightColor().isValid())
cue.setHighlightColor(inbandCue.highlightColor());
+ switch (inbandCue.positionAlign()) {
+ case GenericCueData::Alignment::Start:
+ cue.setPositionAlign(VTTCue::PositionAlignSetting::LineLeft);
+ break;
+ case GenericCueData::Alignment::Middle:
+ cue.setPositionAlign(VTTCue::PositionAlignSetting::Center);
+ break;
+ case GenericCueData::Alignment::End:
+ cue.setPositionAlign(VTTCue::PositionAlignSetting::LineRight);
+ break;
+ case GenericCueData::Alignment::None:
+ break;
+ }
+
if (inbandCue.align() == GenericCueData::Alignment::Start)
cue.setAlign(VTTCue::AlignSetting::Start);
else if (inbandCue.align() == GenericCueData::Alignment::Middle)
diff --git a/Source/WebCore/html/track/TextTrackCueGeneric.cpp b/Source/WebCore/html/track/TextTrackCueGeneric.cpp
index f90bd3b..ba4d4d2 100644
--- a/Source/WebCore/html/track/TextTrackCueGeneric.cpp
+++ b/Source/WebCore/html/track/TextTrackCueGeneric.cpp
@@ -73,91 +73,19 @@
void TextTrackCueGenericBoxElement::applyCSSProperties()
{
+ VTTCueBox::applyCSSProperties();
+
RefPtr<TextTrackCueGeneric> cue = static_cast<TextTrackCueGeneric*>(getCue());
if (!cue)
return;
- setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
- setInlineStyleProperty(CSSPropertyUnicodeBidi, CSSValuePlaintext);
-
Ref<HTMLSpanElement> cueElement = cue->element();
-
- double textPosition = cue->calculateComputedTextPosition();
- double linePosition = cue->calculateComputedLinePosition();
-
- CSSValueID alignment = cue->getCSSAlignment();
- float size = static_cast<float>(cue->getCSSSize());
- if (cue->useDefaultPosition()) {
- setInlineStyleProperty(CSSPropertyBottom, 0, CSSUnitType::CSS_PX);
- setInlineStyleProperty(CSSPropertyMarginBottom, 1.0, CSSUnitType::CSS_PERCENTAGE);
- } else {
- setInlineStyleProperty(CSSPropertyLeft, static_cast<float>(textPosition), CSSUnitType::CSS_PERCENTAGE);
- setInlineStyleProperty(CSSPropertyTop, static_cast<float>(linePosition), CSSUnitType::CSS_PERCENTAGE);
-
- double authorFontSize = cue->baseFontSizeRelativeToVideoHeight();
- if (!authorFontSize)
- authorFontSize = DEFAULTCAPTIONFONTSIZE;
-
- if (cue->fontSizeMultiplier())
- authorFontSize *= cue->fontSizeMultiplier() / 100;
-
- double multiplier = fontSizeFromCaptionUserPrefs() / authorFontSize;
- double newCueSize = std::min(size * multiplier, 100.0);
- if (cue->vertical() == VTTCue::DirectionSetting::Horizontal) {
- setInlineStyleProperty(CSSPropertyWidth, newCueSize, CSSUnitType::CSS_PERCENTAGE);
- if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
- setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(textPosition - (newCueSize - cue->getCSSSize()) / 2), CSSUnitType::CSS_PERCENTAGE);
- } else {
- setInlineStyleProperty(CSSPropertyHeight, newCueSize, CSSUnitType::CSS_PERCENTAGE);
- if ((alignment == CSSValueMiddle || alignment == CSSValueCenter) && multiplier != 1.0)
- setInlineStyleProperty(CSSPropertyTop, static_cast<double>(linePosition - (newCueSize - cue->getCSSSize()) / 2), CSSUnitType::CSS_PERCENTAGE);
- }
- }
-
- double maxSize = 100.0;
-
- if (alignment == CSSValueEnd || alignment == CSSValueRight)
- maxSize = textPosition;
- else if (alignment == CSSValueStart || alignment == CSSValueLeft)
- maxSize = 100.0 - textPosition;
-
- if (cue->vertical() == VTTCue::DirectionSetting::Horizontal) {
- setInlineStyleProperty(CSSPropertyMinWidth, "min-content"_s);
- setInlineStyleProperty(CSSPropertyMaxWidth, maxSize, CSSUnitType::CSS_PERCENTAGE);
- } else {
- setInlineStyleProperty(CSSPropertyMinHeight, "min-content"_s);
- setInlineStyleProperty(CSSPropertyMaxHeight, maxSize, CSSUnitType::CSS_PERCENTAGE);
- }
-
if (cue->foregroundColor().isValid())
cueElement->setInlineStyleProperty(CSSPropertyColor, serializationForHTML(cue->foregroundColor()));
if (cue->highlightColor().isValid())
cueElement->setInlineStyleProperty(CSSPropertyBackgroundColor, serializationForHTML(cue->highlightColor()));
-
- if (cue->vertical() == VTTCue::DirectionSetting::Horizontal)
- setInlineStyleProperty(CSSPropertyHeight, CSSValueAuto);
- else
- setInlineStyleProperty(CSSPropertyWidth, CSSValueAuto);
-
- if (cue->baseFontSizeRelativeToVideoHeight())
- cue->setFontSize(cue->baseFontSizeRelativeToVideoHeight(), false);
-
- if (cue->align() == VTTCue::AlignSetting::Center)
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueCenter);
- else if (cue->align() == VTTCue::AlignSetting::End)
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueEnd);
- else
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueStart);
-
if (cue->backgroundColor().isValid())
setInlineStyleProperty(CSSPropertyBackgroundColor, serializationForHTML(cue->backgroundColor()));
- setInlineStyleProperty(CSSPropertyWritingMode, cue->getCSSWritingMode(), false);
- setInlineStyleProperty(CSSPropertyWhiteSpaceCollapse, CSSValuePreserve);
- setInlineStyleProperty(CSSPropertyTextWrap, CSSValueWrap);
-
- // Make sure shadow or stroke is not clipped.
- setInlineStyleProperty(CSSPropertyOverflow, CSSValueVisible);
- cueElement->setInlineStyleProperty(CSSPropertyOverflow, CSSValueVisible);
}
Ref<TextTrackCueGeneric> TextTrackCueGeneric::create(ScriptExecutionContext& context, const MediaTime& start, const MediaTime& end, const String& content)
@@ -187,18 +115,22 @@
return result;
}
-void TextTrackCueGeneric::setFontSize(int fontSize, bool important)
+void TextTrackCueGeneric::setBaseFontSizeRelativeToVideoHeight(double baseFontSize)
{
- if (!fontSize)
+ if (m_baseFontSizeRelativeToVideoHeight == baseFontSize)
return;
-
- if (important || !hasDisplayTree() || !baseFontSizeRelativeToVideoHeight()) {
- VTTCue::setFontSize(fontSize, important);
- return;
- }
- if (auto* displayTree = displayTreeInternal())
- displayTree->setInlineStyleProperty(CSSPropertyFontSize, fontSize, CSSUnitType::CSS_CQH);
+ m_baseFontSizeRelativeToVideoHeight = baseFontSize;
+ setFontSize(m_baseFontSizeRelativeToVideoHeight * m_fontSizeMultiplier, fontSizeIsImportant());
+}
+
+void TextTrackCueGeneric::setFontSizeMultiplier(double multiplier)
+{
+ if (m_fontSizeMultiplier == multiplier)
+ return;
+
+ m_fontSizeMultiplier = multiplier;
+ setFontSize(m_baseFontSizeRelativeToVideoHeight * m_fontSizeMultiplier, fontSizeIsImportant());
}
bool TextTrackCueGeneric::cueContentsMatch(const TextTrackCue& otherTextTrackCue) const
diff --git a/Source/WebCore/html/track/TextTrackCueGeneric.h b/Source/WebCore/html/track/TextTrackCueGeneric.h
index 6ccf339..8f732ff 100644
--- a/Source/WebCore/html/track/TextTrackCueGeneric.h
+++ b/Source/WebCore/html/track/TextTrackCueGeneric.h
@@ -43,10 +43,10 @@
bool useDefaultPosition() const { return m_useDefaultPosition; }
double baseFontSizeRelativeToVideoHeight() const { return m_baseFontSizeRelativeToVideoHeight; }
- void setBaseFontSizeRelativeToVideoHeight(double size) { m_baseFontSizeRelativeToVideoHeight = size; }
+ void setBaseFontSizeRelativeToVideoHeight(double);
double fontSizeMultiplier() const { return m_fontSizeMultiplier; }
- void setFontSizeMultiplier(double size) { m_fontSizeMultiplier = size; }
+ void setFontSizeMultiplier(double);
const String& fontName() const { return m_fontName; }
void setFontName(const String& name) { m_fontName = name; }
@@ -60,8 +60,6 @@
const Color& highlightColor() const { return m_highlightColor; }
void setHighlightColor(const Color& color) { m_highlightColor = color; }
- void setFontSize(int, bool important) final;
-
private:
TextTrackCueGeneric(Document&, const MediaTime& start, const MediaTime& end, const String&);
diff --git a/Source/WebCore/html/track/VTTCue.h b/Source/WebCore/html/track/VTTCue.h
index ecc115c..bbd243e 100644
--- a/Source/WebCore/html/track/VTTCue.h
+++ b/Source/WebCore/html/track/VTTCue.h
@@ -90,18 +90,13 @@
void applyCSSProperties() override;
- void setFontSizeFromCaptionUserPrefs(int fontSize) { m_fontSizeFromCaptionUserPrefs = fontSize; }
-
protected:
VTTCueBox(Document&, VTTCue&);
RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
- int fontSizeFromCaptionUserPrefs() const { return m_fontSizeFromCaptionUserPrefs; }
-
private:
WeakPtr<VTTCue> m_cue;
- int m_fontSizeFromCaptionUserPrefs;
};
// ----------------------------
diff --git a/Source/WebCore/platform/graphics/InbandGenericCue.cpp b/Source/WebCore/platform/graphics/InbandGenericCue.cpp
index 45d1d24..3da7f15 100644
--- a/Source/WebCore/platform/graphics/InbandGenericCue.cpp
+++ b/Source/WebCore/platform/graphics/InbandGenericCue.cpp
@@ -74,6 +74,25 @@
if (m_cueData.m_position > 0)
object->setDouble("position"_s, m_cueData.m_position);
+ if (m_cueData.m_positionAlign != GenericCueData::Alignment::None) {
+ ASCIILiteral positionAlign = ""_s;
+ switch (m_cueData.m_positionAlign) {
+ case GenericCueData::Alignment::Start:
+ positionAlign = "Start"_s;
+ break;
+ case GenericCueData::Alignment::Middle:
+ positionAlign = "Middle"_s;
+ break;
+ case GenericCueData::Alignment::End:
+ positionAlign = "End"_s;
+ break;
+ case GenericCueData::Alignment::None:
+ positionAlign = "None"_s;
+ break;
+ }
+ object->setString("positionAlign"_s, positionAlign);
+ }
+
if (m_cueData.m_align != GenericCueData::Alignment::None) {
ASCIILiteral align = ""_s;
switch (m_cueData.m_align) {
diff --git a/Source/WebCore/platform/graphics/InbandGenericCue.h b/Source/WebCore/platform/graphics/InbandGenericCue.h
index 96a7dea..322dcc8 100644
--- a/Source/WebCore/platform/graphics/InbandGenericCue.h
+++ b/Source/WebCore/platform/graphics/InbandGenericCue.h
@@ -39,7 +39,7 @@
enum class Status : uint8_t { Uninitialized, Partial, Complete };
GenericCueData() = default;
- GenericCueData(InbandGenericCueIdentifier uniqueId, const MediaTime& startTime, const MediaTime& endTime, const AtomString& id, const String& content, const String& fontName, double line, double position, double size, double baseFontSize, double relativeFontSize, const Color& foregroundColor, const Color& backgroundColor, const Color& highlightColor, GenericCueData::Alignment align, GenericCueData::Status status)
+ GenericCueData(InbandGenericCueIdentifier uniqueId, const MediaTime& startTime, const MediaTime& endTime, const AtomString& id, const String& content, const String& fontName, double line, double position, double size, double baseFontSize, double relativeFontSize, const Color& foregroundColor, const Color& backgroundColor, const Color& highlightColor, GenericCueData::Alignment positionAlign, GenericCueData::Alignment align, GenericCueData::Status status)
: m_uniqueId(uniqueId)
, m_startTime(startTime)
, m_endTime(endTime)
@@ -54,6 +54,7 @@
, m_foregroundColor(foregroundColor)
, m_backgroundColor(backgroundColor)
, m_highlightColor(highlightColor)
+ , m_positionAlign(positionAlign)
, m_align(align)
, m_status(status)
{
@@ -77,6 +78,7 @@
Color m_foregroundColor;
Color m_backgroundColor;
Color m_highlightColor;
+ Alignment m_positionAlign { Alignment::None };
Alignment m_align { Alignment::None };
Status m_status { Status::Uninitialized };
};
@@ -106,6 +108,9 @@
double position() const { return m_cueData.m_position; }
void setPosition(double position) { m_cueData.m_position = position; }
+ GenericCueData::Alignment positionAlign() const { return m_cueData.m_positionAlign; }
+ void setPositionAlign(GenericCueData::Alignment align) { m_cueData.m_positionAlign = align; }
+
double size() const { return m_cueData.m_size; }
void setSize(double size) { m_cueData.m_size = size; }
diff --git a/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp b/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp
index 7e9f4491..ff44f5d 100644
--- a/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp
+++ b/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp
@@ -341,10 +341,9 @@
cueData->setStartTime(time);
cueData->setEndTime(MediaTime::positiveInfiniteTime());
- // AVFoundation cue "position" is to the center of the text so adjust relative to the edge because we will use it to
- // set CSS "left".
- if (cueData->position() >= 0 && cueData->size() > 0)
- cueData->setPosition(cueData->position() - cueData->size() / 2);
+ // AVFoundation cue "position" is to the center of the text so set "positionAlign" to "middle"
+ // to indicate to VTTCue that this cue should be positioned relative to its center
+ cueData->setPositionAlign(GenericCueData::Alignment::Middle);
cueData->setStatus(GenericCueData::Status::Partial);
diff --git a/Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in b/Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
index cd151ff..6039b2e 100644
--- a/Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
+++ b/Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
@@ -4283,6 +4283,7 @@
WebCore::Color m_foregroundColor;
WebCore::Color m_backgroundColor;
WebCore::Color m_highlightColor;
+ WebCore::GenericCueData::Alignment m_positionAlign;
WebCore::GenericCueData::Alignment m_align;
WebCore::GenericCueData::Status m_status;
};
