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

Issue 2710713003: Media Controls: Remove download button for infinite streams (Closed)

Created:
3 years, 10 months ago by johnme
Modified:
3 years, 10 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, posciak+watch_chromium.org, nessy, Srirama
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Media Controls: Remove download button for infinite streams Specifically, removes the download button for media with infinite duration, since there is no clear end at which to finish the download. Also adds tests for the download button being hidden for empty URLs and HLS streams, and fixes the test for the download button being hidden when hideDownloadUI setting is disabled by making its video non-local. BUG=691561, 654507, 650738 Review-Url: https://codereview.chromium.org/2710713003 Cr-Commit-Position: refs/heads/master@{#451852} Committed: https://chromium.googlesource.com/chromium/src/+/b6920f2e6be77a44ee16b052b45b2c4228854ae0

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+74 lines, -24 lines) Patch
A + third_party/WebKit/LayoutTests/http/tests/media/video-controls-download-button-not-displayed-hide-download-ui.html View 1 chunk +3 lines, -3 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/video-controls-hide-download-ui.html View 1 chunk +0 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp View 2 chunks +6 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/shadow/MediaControlsTest.cpp View 5 chunks +65 lines, -1 line 0 comments Download

Messages

Total messages: 13 (9 generated)
johnme
3 years, 10 months ago (2017-02-21 17:02:30 UTC) #3
mlamouri (slow - plz ping)
lgtm
3 years, 10 months ago (2017-02-21 21:35:59 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/2710713003/1
3 years, 10 months ago (2017-02-21 21:37:58 UTC) #10
commit-bot: I haz the power
3 years, 10 months ago (2017-02-21 22:17:55 UTC) #13
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/b6920f2e6be77a44ee16b052b45b...

Powered by Google App Engine
This is Rietveld 408576698