REGRESSION: Inline media scrubbing always pauses the video
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2015 00:02:50 +0000 (00:02 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2015 00:02:50 +0000 (00:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=146819
<rdar://problem/21572027>

Reviewed by Eric Carlson. Joseph Pecoraro also was really
helpful in diagnosing the problem.

When we moved some code from a getter/setter in the child
class to the base class, it was no longer being called due
to the bad way we were implementing inheritance. The solution
was to have the child class explicitly call into the base
class.

The much better solution would have been to rewrite everything
to use ES6 classes or, as a smaller change, assign the __proto__
directly on the child prototype. But I felt that was a bit
too risky at this point.

* Modules/mediacontrols/mediaControlsApple.js:
(Controller.prototype.extend): Describe in a comment why the extend function
is not suitable.
* Modules/mediacontrols/mediaControlsiOS.js: Add a getter/setter for
scrubbing that calls into the base Controller.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@186652 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js

index bcabe53..e628ebd 100644 (file)
@@ -1,3 +1,29 @@
+2015-07-09  Dean Jackson  <dino@apple.com>
+
+        REGRESSION: Inline media scrubbing always pauses the video
+        https://bugs.webkit.org/show_bug.cgi?id=146819
+        <rdar://problem/21572027>
+
+        Reviewed by Eric Carlson. Joseph Pecoraro also was really
+        helpful in diagnosing the problem.
+
+        When we moved some code from a getter/setter in the child
+        class to the base class, it was no longer being called due
+        to the bad way we were implementing inheritance. The solution
+        was to have the child class explicitly call into the base
+        class.
+
+        The much better solution would have been to rewrite everything
+        to use ES6 classes or, as a smaller change, assign the __proto__
+        directly on the child prototype. But I felt that was a bit
+        too risky at this point.
+
+        * Modules/mediacontrols/mediaControlsApple.js:
+        (Controller.prototype.extend): Describe in a comment why the extend function
+        is not suitable.
+        * Modules/mediacontrols/mediaControlsiOS.js: Add a getter/setter for
+        scrubbing that calls into the base Controller.
+
 2015-07-09  Chris Fleizach  <cfleizach@apple.com>
 
         AX: <details> element should allow expand/close through AX API
 2015-07-09  Chris Fleizach  <cfleizach@apple.com>
 
         AX: <details> element should allow expand/close through AX API
index fa6a358..46e13c8 100644 (file)
@@ -132,6 +132,12 @@ Controller.prototype = {
 
     extend: function(child)
     {
 
     extend: function(child)
     {
+        // This function doesn't actually do what we want it to. In particular it
+        // is not copying the getters and setters to the child class, since they are
+        // not enumerable. What we should do is use ES6 classes, or assign the __proto__
+        // directly.
+        // FIXME: Use ES6 classes.
+
         for (var property in this) {
             if (!child.hasOwnProperty(property))
                 child[property] = this[property];
         for (var property in this) {
             if (!child.hasOwnProperty(property))
                 child[property] = this[property];
index edcc795..1abb2b6 100644 (file)
@@ -630,6 +630,21 @@ ControllerIOS.prototype = {
         return Controller.prototype.controlsAlwaysVisible.call(this);
     },
 
         return Controller.prototype.controlsAlwaysVisible.call(this);
     },
 
+    // Due to the bad way we are faking inheritance here, in particular the extends method
+    // on Controller.prototype, we don't copy getters and setters from the prototype. This
+    // means we have to implement them again, here in the subclass.
+    // FIXME: Use ES6 classes!
+
+    get scrubbing()
+    {
+        return Object.getOwnPropertyDescriptor(Controller.prototype, "scrubbing").get.call(this);
+    },
+
+    set scrubbing(flag)
+    {
+        Object.getOwnPropertyDescriptor(Controller.prototype, "scrubbing").set.call(this, flag);
+    },
+
     get pageScaleFactor()
     {
         return this._pageScaleFactor;
     get pageScaleFactor()
     {
         return this._pageScaleFactor;