Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 2725893002: Media Controls timeline: immediately update current time display (Closed)

Created:
3 years, 9 months ago by johnme
Modified:
3 years, 9 months ago
CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, nessy, Srirama
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Media Controls timeline: immediately update current time display Before this patch, dragging the scrubber starting from its current position would immediately update the current time display, but all other ways of changing the scrubber (dragging from some other start position, click on a position, or using the keyboard) would only update the current time display once the media had finished seeking to the desired position (which might have to wait for network requests). This patch removes that distinction, and now immediately updates the current time display whenever the timeline receives input. This should make it easier to precisely set the current time. BUG=695459, 699096 Review-Url: https://codereview.chromium.org/2725893002 Cr-Commit-Position: refs/heads/master@{#455158} Committed: https://chromium.googlesource.com/chromium/src/+/792dd3474f7076b0dc33126a90ba4affbaac2c7f

Patch Set 1 #

Total comments: 2

Patch Set 2 : Comment nit #

Patch Set 3 : Mark media/controls-drag-timebar-rendering.html failing, see crbug.com/699096 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -12 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp View 1 2 3 chunks +34 lines, -9 lines 0 comments Download

Messages

Total messages: 19 (11 generated)
johnme
3 years, 9 months ago (2017-03-01 16:43:39 UTC) #2
mlamouri (slow - plz ping)
lgtm with comment applied https://codereview.chromium.org/2725893002/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp File third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp (right): https://codereview.chromium.org/2725893002/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp#newcode458 third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp:458: // update synchronously (crbug.com/695459). No ...
3 years, 9 months ago (2017-03-02 10:37:19 UTC) #3
johnme
Addressed nit - thanks! https://codereview.chromium.org/2725893002/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp File third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp (right): https://codereview.chromium.org/2725893002/diff/1/third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp#newcode458 third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp:458: // update synchronously (crbug.com/695459). On ...
3 years, 9 months ago (2017-03-02 13:31:30 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2725893002/20001
3 years, 9 months ago (2017-03-02 13:32:03 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/399285)
3 years, 9 months ago (2017-03-02 14:40:10 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2725893002/40001
3 years, 9 months ago (2017-03-07 14:41:48 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2725893002/40001
3 years, 9 months ago (2017-03-07 14:57:47 UTC) #16
commit-bot: I haz the power
3 years, 9 months ago (2017-03-07 18:50:46 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/792dd3474f7076b0dc33126a90ba...

Powered by Google App Engine
This is Rietveld 408576698