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

Unified Diff: Source/core/layout/LayoutTheme.cpp

Issue 1156993013: New media playback UI. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: fixed some tests from previous CL. Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/layout/LayoutTheme.cpp
diff --git a/Source/core/layout/LayoutTheme.cpp b/Source/core/layout/LayoutTheme.cpp
index 07a631b00b2996bd8eed90eb249777df915f648c..479a9edf65135fbc4e87cef23a737eb34a7ac5ca 100644
--- a/Source/core/layout/LayoutTheme.cpp
+++ b/Source/core/layout/LayoutTheme.cpp
@@ -228,7 +228,7 @@ String LayoutTheme::extraDefaultStyleSheet()
return runtimeCSS.toString();
}
-static String formatChromiumMediaControlsTime(float time, float duration)
+static String formatChromiumMediaControlsTime(float time, float duration, bool includeSeparator)
{
if (!std::isfinite(time))
time = 0;
@@ -236,30 +236,49 @@ static String formatChromiumMediaControlsTime(float time, float duration)
duration = 0;
int seconds = static_cast<int>(fabsf(time));
int hours = seconds / (60 * 60);
- int minutes = (seconds / 60) % 60;
+ int minutes = (seconds / 60);
+ if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled())
+ minutes %= 60;
philipj_slow 2015/07/09 13:40:56 This makes it somewhat hard to follow the differen
liberato (no reviews please) 2015/07/09 22:35:35 Done.
+
seconds %= 60;
// duration defines the format of how the time is rendered
int durationSecs = static_cast<int>(fabsf(duration));
- int durationHours = durationSecs / (60 * 60);
- int durationMins = (durationSecs / 60) % 60;
+ int durationMins = (durationSecs / 60);
+
+ if (!RuntimeEnabledFeatures::newMediaPlaybackUiEnabled()) {
+ int durationHours = durationSecs / (60 * 60);
+ durationMins %= 60;
+ if (durationHours || hours)
+ return String::format("%s%01d:%02d:%02d", (time < 0 ? "-" : ""), hours, minutes, seconds);
+ if (durationMins > 9)
+ return String::format("%s%02d:%02d", (time < 0 ? "-" : ""), minutes, seconds);
+
+ return String::format("%s%01d:%02d", (time < 0 ? "-" : ""), minutes, seconds);
+ }
+
+ // New UI includes a leading "/ " before duration.
+ const char* separator = (includeSeparator ? "/ " : "");
philipj_slow 2015/07/09 13:40:56 I don't think the () are required here.
liberato (no reviews please) 2015/07/09 22:35:35 Done.
- if (durationHours || hours)
- return String::format("%s%01d:%02d:%02d", (time < 0 ? "-" : ""), hours, minutes, seconds);
- if (durationMins > 9)
- return String::format("%s%02d:%02d", (time < 0 ? "-" : ""), minutes, seconds);
+ // 0-9 minutes duration is 0:00
+ // 10-60 minutes duration is 00:00
philipj_slow 2015/07/09 13:40:56 Shouldn't this be 10-99? A 61 minute movie shouldn
liberato (no reviews please) 2015/07/09 22:35:35 i've asked the UX designers to verify the intentio
liberato (no reviews please) 2015/07/14 22:10:36 sorry, forgot to update this one. the 61:00 is in
+ // >60 minutes duration is 000:00
+ if (durationMins > 60 || minutes > 60)
+ return String::format("%s%s%03d:%02d", separator, (time < 0 ? "-" : ""), minutes, seconds);
+ if (durationMins > 10)
+ return String::format("%s%s%02d:%02d", separator, (time < 0 ? "-" : ""), minutes, seconds);
- return String::format("%s%01d:%02d", (time < 0 ? "-" : ""), minutes, seconds);
+ return String::format("%s%s%01d:%02d", separator, (time < 0 ? "-" : ""), minutes, seconds);
}
String LayoutTheme::formatMediaControlsTime(float time) const
{
- return formatChromiumMediaControlsTime(time, time);
+ return formatChromiumMediaControlsTime(time, time, true);
}
String LayoutTheme::formatMediaControlsCurrentTime(float currentTime, float duration) const
{
- return formatChromiumMediaControlsTime(currentTime, duration);
+ return formatChromiumMediaControlsTime(currentTime, duration, false);
}
Color LayoutTheme::activeSelectionBackgroundColor() const

Powered by Google App Engine
This is Rietveld 408576698