1160656 - updating the video dimensions cache on one view should not change caches on other views

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Found this while noticing that the views in some ui-showcase iframes were interfering with views in others.  After a bunch of hunting, I learned about a corner of JS that I hadn't found myself in before: if you put a non-simple object onto the prototype, and then assign _inside_ that object from a concrete object derived from the prototype, the assignment changes the prototype, not the derived object.  Good times!

In other words, updating the video dimensions cache on one StandaloneRoomView was changing them on all the others, which was making the ui-showcase unworkable.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Separate MediaSetupMixin caches on different instances more completel

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8600480 [details] [diff] [review]
Separate MediaSetupMixin caches on different instances more completel

Review of attachment 8600480 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent! Thanks for this!

::: browser/components/loop/test/shared/mixins_test.js
@@ +406,5 @@
>              .eql(0.32857142857142857);
>          });
> +
> +        it("should not populate the cache on another component instance",
> +          function() {

nit: I think it's good to not be constrained by strictly following the 80 chars rule, but take a more flexible and modern approach of 80-120 chars and apply line-breaks in that range in service of readability.

I have two vertical guides set in my editor; one on 80ch and another on 120ch.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

https://hg.mozilla.org/integration/fx-team/rev/2eeddac30370

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2eeddac30370