Bug 42677

Summary: Chrome browser Bug: Pause button stays when <audio> hits end
Product: WebKit Reporter: Pranav Kedia <pranavk>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ajwong, commit-queue, dimich, pranavk
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://code.google.com/p/chromium/issues/detail?id=21739
Attachments:
Description Flags
Proposed patch.
none
New patch based on feedback.
dimich: review-
New revised patch after fresh enlistment in WebKit. The path issues should be resolved. none

Description Pranav Kedia 2010-07-20 15:01:31 PDT
This is a copy of bug 21739 on Chrome browser. Please see http://code.google.com/p/chromium/issues/detail?id=21739. Here is the description taken from the bug 21739:

In chrome browser, go to http://en.wikipedia.org/wiki/The_Star-Spangled_Banner and click "play" on one of the media files (right hand side of screen). Notice that when the file hits the end, the "pause" button stays a "pause" button rather than turning into "play". This seems kinda broken. (I would expect the slider to return to the beginning and the button to turn to "play").

The fix of this bug is in WebKit/WebCore/rendering/RenderMediaControlsChromium.cpp. Hence filing this bug. Here is the diff of the fix:

pranavk@pranavk-l1:/usr/local/google/home/pranavk/chrome/src/third_party/WebKit/WebCore$ svn diff
Index: rendering/RenderMediaControlsChromium.cpp
===================================================================
--- rendering/RenderMediaControlsChromium.cpp	(revision 62294)
+++ rendering/RenderMediaControlsChromium.cpp	(working copy)
@@ -96,7 +96,7 @@
     if (!hasSource(mediaElement))
         return paintMediaButton(paintInfo.context, rect, mediaPlayDisabled);
 
-    return paintMediaButton(paintInfo.context, rect, mediaElement->paused() ? mediaPlay : mediaPause);
+    return paintMediaButton(paintInfo.context, rect, mediaElement->canPlay() ? mediaPlay : mediaPause);
 }
 
 static Image* getMediaSliderThumb()
pranavk@pranavk-l1:/usr/local/google/home/pranavk/chrome/src/third_party/WebKit/WebCore$
Comment 1 Pranav Kedia 2010-07-22 15:57:50 PDT
Created attachment 62352 [details]
Proposed patch.
Comment 2 Pranav Kedia 2010-07-22 17:06:30 PDT
Created attachment 62363 [details]
New patch based on feedback.
Comment 3 Dmitry Titov 2010-07-22 17:13:31 PDT
Comment on attachment 62363 [details]
New patch based on feedback.

> Index: rendering/RenderMediaControlsChromium.cpp

It seems the patch was created while current directory was different from the root WebKit one. I'm not sure commit-queue will be able to deal with it.
Please re-submit patch created from WebKit root (where WebCore is a subdir)
Comment 4 Pranav Kedia 2010-07-22 17:44:24 PDT
This proposed fix has been taken from the function MediaControlPlayButtonElement::updateDisplayType() in http://trac.webkit.org/browser/trunk/WebCore/rendering/MediaControlElements.cpp
Comment 5 Pranav Kedia 2010-07-26 15:25:04 PDT
Created attachment 62620 [details]
New revised patch after fresh enlistment in WebKit. The path issues should be resolved.
Comment 6 Adam Barth 2010-07-27 07:50:58 PDT
Comment on attachment 62620 [details]
New revised patch after fresh enlistment in WebKit. The path issues should be resolved.

Nice.  Thanks.  Do we need to update the test_expectations.txt?
Comment 7 WebKit Commit Bot 2010-07-27 08:06:30 PDT
Comment on attachment 62620 [details]
New revised patch after fresh enlistment in WebKit. The path issues should be resolved.

Clearing flags on attachment: 62620

Committed r64128: <http://trac.webkit.org/changeset/64128>
Comment 8 WebKit Commit Bot 2010-07-27 08:06:35 PDT
All reviewed patches have been landed.  Closing bug.