Bug 24063 - Allow port to require a user gesture to play/pause an <audio> or <video> element
Summary: Allow port to require a user gesture to play/pause an <audio> or <video> element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-20 11:11 PST by Eric Carlson
Modified: 2009-02-20 13:13 PST (History)
0 users

See Also:


Attachments
proposed patch (6.38 KB, patch)
2009-02-20 11:15 PST, Eric Carlson
simon.fraser: review-
Details | Formatted Diff | Diff
revised patch (7.71 KB, patch)
2009-02-20 12:39 PST, Eric Carlson
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-02-20 11:11:59 PST
A port should be able to require a script to be called from a user gesture to start/stop a media element .
Comment 1 Eric Carlson 2009-02-20 11:15:04 PST
Created attachment 27833 [details]
proposed patch
Comment 2 Simon Fraser (smfr) 2009-02-20 12:01:53 PST
Comment on attachment 27833 [details]
proposed patch

> Index: WebCore/html/HTMLMediaElement.cpp
> ===================================================================

>  void HTMLMediaElement::loadTimerFired(Timer<HTMLMediaElement>*)
>  {
>      ExceptionCode ec;
> +
> +    ++m_internalCall;
>      load(ec);
> +    --m_internalCall;

I think it would be better for load() to wrap a loadInternal().
load() could do the permissions checking, and loadInternal() should
always load. Same with play() and pause().

> Index: WebCore/html/HTMLMediaElement.h
> ===================================================================

> +    enum BehaviorRestrictions 
>      { 
> -        NoLoadRestriction = 0,
> -        RequireUserGestureLoadRestriction = 1 << 0, 
> +        NoRestrictions = 0,
> +        RequireUserGestureLoadRestriction = 1 << 0,

RequireUserGestureForLoadRestriction
> +        RequireUserGestureRateChangeRestriction = 1 << 1,
RequireUserGestureForRateChangeRestriction

>      };
>  
>  
> @@ -237,7 +238,7 @@ protected:
>  
>      OwnPtr<MediaPlayer> m_player;
>  
> -    LoadRestrictions m_loadRestrictions;
> +    BehaviorRestrictions m_Restrictions;

m_Restrictions -> m_restrictions
Comment 3 Eric Carlson 2009-02-20 12:39:53 PST
Created attachment 27836 [details]
revised patch
Comment 4 Simon Fraser (smfr) 2009-02-20 12:46:38 PST
Comment on attachment 27836 [details]
revised patch

> Index: WebCore/html/HTMLMediaElement.cpp
> ===================================================================

>  void HTMLMediaElement::togglePlayState(ExceptionCode& ec)
>  {
>      if (canPlay())
> -        play(ec);
> +        playInternal(ec);
>      else 
> -        pause(ec);
> +        pauseInternal(ec);
>  }

Maybe add a comment to say why it's always OK to call the
internal methods.

> Index: WebCore/html/HTMLMediaElement.h
> ===================================================================

> +    void loadInternal(ExceptionCode& ec);
> +    void playInternal(ExceptionCode& ec);
> +    void pauseInternal(ExceptionCode& ec);

Add a comment to say why these are needed.
Comment 5 Eric Carlson 2009-02-20 13:13:40 PST
Committed revision 41117.