Skip to content

Commit

Permalink
Support negative timestamps of TextTrackCue
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=93143
rdar://problem/113035924

Reviewed by Eric Carlson.

This patch aligns WebKit with Blink / Chromium, Gecko / Firefox and Web-Spec.

Merge: https://chromium-review.googlesource.com/c/chromium/src/+/863270

Ensure proper behaviour for negative timestamps of TextTrackCue.
1. Cues with negative startTime should become active from 0s.
2. Cues with negative startTime and endTime should never be active.

Web-Spec References: whatwg/html@af1b8e2 and
https://www.w3.org/Bugs/Public/show_bug.cgi?id=18480

* Source/WebCore/html/track/TextTrack.cpp:
(TextTrack::addCue): Remove 'TODO' and greater then 'zeroTime' case
* Source/WebCore/html/track/TextTrackCue.cpp:
(TextTrackCue::setStartTime): Remove 'TODO' and remove 'value' less than 0 early return case
(TextTrackCue::setEndTime): Ditto
* Source/WebCore/html/track/TextTrackCueList.cpp:
(TextTrackCueList::add): Remove 'ASSERT' for 'non-negative' case
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/track/track-element/track-cue-negative-timestamp-events-expected.txt: Rebaselined
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/track/track-element/track-cue-negative-duration-expected.txt: Rebaselined
* LayoutTests/media/track/track-cue-negative-timestamp.html: Removed in favor of WPT
* LayoutTests/media/track/track-cue-negative-timestamp-expected.txt: Ditto

Canonical link: https://commits.webkit.org/267042@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed Aug 18, 2023
1 parent b93351e commit fb0116d
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 80 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@


FAIL Enter, Exit events for a cue with negative duration assert_equals: expected 1 but got 0
PASS Enter, Exit events for a cue with negative duration

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@


FAIL Enter, Exit events for cues with negative timestamps assert_equals: expected 1 but got 0
PASS Enter, Exit events for cues with negative timestamps

24 changes: 0 additions & 24 deletions LayoutTests/media/track/track-cue-negative-timestamp-expected.txt

This file was deleted.

44 changes: 0 additions & 44 deletions LayoutTests/media/track/track-cue-negative-timestamp.html

This file was deleted.

5 changes: 2 additions & 3 deletions Source/WebCore/html/track/TextTrack.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2011, 2013 Google Inc. All rights reserved.
* Copyright (C) 2011-2018 Google Inc. All rights reserved.
* Copyright (C) 2011-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -327,8 +327,7 @@ ExceptionOr<void> TextTrack::addCue(Ref<TextTrackCue>&& cue)

INFO_LOG(LOGIDENTIFIER, cue.get());

// TODO(93143): Add spec-compliant behavior for negative time values.
if (!cue->startMediaTime().isValid() || !cue->endMediaTime().isValid() || cue->startMediaTime() < MediaTime::zeroTime() || cue->endMediaTime() < MediaTime::zeroTime())
if (!cue->startMediaTime().isValid() || !cue->endMediaTime().isValid())
return { };

// 4.8.10.12.5 Text track API
Expand Down
8 changes: 3 additions & 5 deletions Source/WebCore/html/track/TextTrackCue.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2011, 2013 Google Inc. All rights reserved.
* Copyright (C) 2011-2018 Google Inc. All rights reserved.
* Copyright (C) 2011-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -279,8 +279,7 @@ void TextTrackCue::setId(const AtomString& id)

void TextTrackCue::setStartTime(double value)
{
// TODO(93143): Add spec-compliant behavior for negative time values.
if (m_startTime.toDouble() == value || value < 0)
if (m_startTime.toDouble() == value)
return;

setStartTime(MediaTime::createWithDouble(value));
Expand All @@ -295,8 +294,7 @@ void TextTrackCue::setStartTime(const MediaTime& value)

void TextTrackCue::setEndTime(double value)
{
// TODO(93143): Add spec-compliant behavior for negative time values.
if (m_endTime.toDouble() == value || value < 0)
if (m_endTime.toDouble() == value)
return;

setEndTime(MediaTime::createWithDouble(value));
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/html/track/TextTrackCueList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ TextTrackCueList& TextTrackCueList::activeCues()
void TextTrackCueList::add(Ref<TextTrackCue>&& cue)
{
ASSERT(!m_vector.contains(cue.ptr()));
ASSERT(cue->startMediaTime() >= MediaTime::zeroTime());
ASSERT(cue->endMediaTime() >= MediaTime::zeroTime());

RefPtr<TextTrackCue> cueRefPtr { WTFMove(cue) };
unsigned insertionPosition = std::upper_bound(m_vector.begin(), m_vector.end(), cueRefPtr, cueSortsBefore) - m_vector.begin();
Expand Down

0 comments on commit fb0116d

Please sign in to comment.