Skip to content
Permalink
Browse files
[Mac, iOS] Captions are appearing multiple times during repeated vide…
…o play through

https://bugs.webkit.org/show_bug.cgi?id=135680
Source/WebCore:

<rdar://problem/17926802>

Reviewed by Eric Carlson.

Test: media/track/track-in-band-cues-added-once.html

Revert TextTrackCueGeneric::isOrderedBefore logic to its original form, and add
a new 'isOrderedBeforeDuringDisplay' for the special case of displaying captions.

* html/shadow/MediaControlElements.cpp:
(WebCore::compareCueIntervalForDisplay): Added helper function.
(WebCore::MediaControlTextTrackContainerElement::updateDisplay): Use the new
'isOrderedBeforeDuringDisplay' to order the cues for display.
* html/track/TextTrackCue.h:
(WebCore::TextTrackCue::isOrderedBeforeDuringDisplay): Added. This just
calls the existing 'isOrderedBefore' method.
* html/track/TextTrackCueGeneric.cpp:
(WebCore::TextTrackCueGeneric::isOrderedBefore): Revert to logic used
prior to r171700.
(WebCore::TextTrackCueGeneric::isOrderedBeforeDuringDisplay): New method that
implements the behavior in r171700.
* html/track/TextTrackCueGeneric.h:

LayoutTests:

<rdar://problem/17926802>

Reviewed by Eric Carlson.

Reactivate the 'track-in-band-cues-added-once.html' test. We would have caught
this bug immediately if the test had been enabled.

* platform/mac/TestExpectations: Turn 'track-in-band-cues-added-once.html' back
on.


Canonical link: https://commits.webkit.org/153574@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172213 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
brentfulgham committed Aug 7, 2014
1 parent 160a595 commit a3c07c89132bd5ed0374453a218982075804ebf4
Showing 7 changed files with 69 additions and 3 deletions.
@@ -1,3 +1,17 @@
2014-08-06 Brent Fulgham <bfulgham@apple.com>

[Mac, iOS] Captions are appearing multiple times during repeated video play through
https://bugs.webkit.org/show_bug.cgi?id=135680
<rdar://problem/17926802>

Reviewed by Eric Carlson.

Reactivate the 'track-in-band-cues-added-once.html' test. We would have caught
this bug immediately if the test had been enabled.

* platform/mac/TestExpectations: Turn 'track-in-band-cues-added-once.html' back
on.

2014-08-07 Michał Pakuła vel Rutka <m.pakula@samsung.com>

Unreviewed EFL gardening
@@ -1195,8 +1195,6 @@ webkit.org/b/123489 [ Mavericks ] inspector-protocol/debugger/setBreakpoint-opti

webkit.org/b/123490 [ Mavericks ] webaudio/oscillator-sawtooth.html [ Failure ]

webkit.org/b/123498 [ Mavericks ] media/track/track-in-band-cues-added-once.html [ Failure ]

webkit.org/b/123522 [ Mavericks ] media/track/track-in-band-legacy-api.html [ Pass Failure ]

# Audio and video tracks aren't supported on mac
@@ -1,3 +1,30 @@
2014-08-06 Brent Fulgham <bfulgham@apple.com>

[Mac, iOS] Captions are appearing multiple times during repeated video play through
https://bugs.webkit.org/show_bug.cgi?id=135680
<rdar://problem/17926802>

Reviewed by Eric Carlson.

Test: media/track/track-in-band-cues-added-once.html

Revert TextTrackCueGeneric::isOrderedBefore logic to its original form, and add
a new 'isOrderedBeforeDuringDisplay' for the special case of displaying captions.

* html/shadow/MediaControlElements.cpp:
(WebCore::compareCueIntervalForDisplay): Added helper function.
(WebCore::MediaControlTextTrackContainerElement::updateDisplay): Use the new
'isOrderedBeforeDuringDisplay' to order the cues for display.
* html/track/TextTrackCue.h:
(WebCore::TextTrackCue::isOrderedBeforeDuringDisplay): Added. This just
calls the existing 'isOrderedBefore' method.
* html/track/TextTrackCueGeneric.cpp:
(WebCore::TextTrackCueGeneric::isOrderedBefore): Revert to logic used
prior to r171700.
(WebCore::TextTrackCueGeneric::isOrderedBeforeDuringDisplay): New method that
implements the behavior in r171700.
* html/track/TextTrackCueGeneric.h:

2014-08-07 Jer Noble <jer.noble@apple.com>

[Mac] Taking a paused video full screen flashes black at beginning of animation.
@@ -1096,6 +1096,11 @@ RenderPtr<RenderElement> MediaControlTextTrackContainerElement::createElementRen
return createRenderer<RenderTextTrackContainerElement>(*this, WTF::move(style));
}

static bool compareCueIntervalForDisplay(const CueInterval& one, const CueInterval& two)
{
return one.data()->isPositionedAbove(two.data());
};

void MediaControlTextTrackContainerElement::updateDisplay()
{
if (!mediaController()->closedCaptionsVisible())
@@ -1149,7 +1154,12 @@ void MediaControlTextTrackContainerElement::updateDisplay()
// them so that the new cue is at the bottom.
if (childNodeCount() < activeCues.size())
removeChildren();


// Sort the active cues for the appropriate display order. For example, for roll-up
// or paint-on captions, we need to add the cues in reverse chronological order,
// so that the newest captions appear at the bottom.
std::sort(activeCues.begin(), activeCues.end(), &compareCueIntervalForDisplay);

// 10. For each text track cue cue in cues that has not yet had
// corresponding CSS boxes added to output, in text track cue order, run the
// following substeps:
@@ -82,6 +82,7 @@ class TextTrackCue : public RefCounted<TextTrackCue>, public EventTargetWithInli
virtual ScriptExecutionContext* scriptExecutionContext() const override final { return &m_scriptExecutionContext; }

virtual bool isOrderedBefore(const TextTrackCue*) const;
virtual bool isPositionedAbove(const TextTrackCue* cue) const { return isOrderedBefore(cue); }

bool hasEquivalentStartTime(const TextTrackCue&) const;

@@ -202,13 +202,28 @@ bool TextTrackCueGeneric::doesExtendCue(const TextTrackCue& cue) const

bool TextTrackCueGeneric::isOrderedBefore(const TextTrackCue* that) const
{
if (VTTCue::isOrderedBefore(that))
return true;

if (that->cueType() == Generic && startTime() == that->startTime() && endTime() == that->endTime()) {
// Further order generic cues by their calculated line value.
std::pair<double, double> thisPosition = getPositionCoordinates();
std::pair<double, double> thatPosition = toVTTCue(that)->getPositionCoordinates();
return thisPosition.second > thatPosition.second || (thisPosition.second == thatPosition.second && thisPosition.first < thatPosition.first);
}

return false;
}

bool TextTrackCueGeneric::isPositionedAbove(const TextTrackCue* that) const
{
if (that->cueType() == Generic && startTime() == that->startTime() && endTime() == that->endTime()) {
// Further order generic cues by their calculated line value.
std::pair<double, double> thisPosition = getPositionCoordinates();
std::pair<double, double> thatPosition = toVTTCue(that)->getPositionCoordinates();
return thisPosition.second > thatPosition.second || (thisPosition.second == thatPosition.second && thisPosition.first < thatPosition.first);
}

if (that->cueType() == Generic)
return startTime() > that->startTime();

@@ -80,6 +80,7 @@ class TextTrackCueGeneric final : public VTTCue {

private:
virtual bool isOrderedBefore(const TextTrackCue*) const override;
virtual bool isPositionedAbove(const TextTrackCue*) const override;

TextTrackCueGeneric(ScriptExecutionContext&, double start, double end, const String&);

0 comments on commit a3c07c8

Please sign in to comment.