1478875 - add different default layout for autoplay prompt

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Alastor Wu [:alwu]]

In our shield-study, we want to provide different layout of prompt to different user groups.

We need 4 different layouts for the default prompt.
1) [Allow], [Remember this decision] unchecked
2) [Don’t Allow], [Remember this decision] unchecked
3) [Allow], [Remember this decision] checked
4) [Don’t Allow], [Remember this decision] checked

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1478875 - add different default layout for autoplay prompt.

In our shield-study, we want to provide different layout of prompt to different user groups.

We need 4 different layouts for the default prompt.
0) [Allow], [Remember this decision] unchecked
1) [Don’t Allow], [Remember this decision] unchecked
2) [Allow], [Remember this decision] checked
3) [Don’t Allow], [Remember this decision] checked

Review commit: https://reviewboard.mozilla.org/r/259832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259832/

Comment 2

[Dale Harvey (:daleharvey)]

Comment on attachment 8995377 [details]
Bug 1478875 - add different default layout for autoplay prompt.

https://reviewboard.mozilla.org/r/259832/#review267020

I am not actually a peer here so this will need a peer review, johannh is who I have been asking but is on PTO at the moment, before the 31st I believe r=florian is best to check on this

::: browser/modules/PermissionUI.jsm:843
(Diff revision 1)
> +    if (layout == 0 || layout == 2) {
> +      actions.push(allowAction, denyAction);
> +    } else {
> +      actions.push(denyAction, allowAction);
> +    }
> +    return actions;

nit

    if (layout == 0 || layout == 2) {
      return [allowAction, denyAction];
     } 
     return [denyAction, allowAction];

Seems a tad cleaner imo, although wouldnt block on it

Comment 3

[Alastor Wu [:alwu]]

Comment on attachment 8995377 [details]
Bug 1478875 - add different default layout for autoplay prompt.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259832/diff/1-2/

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 8995377 [details]
Bug 1478875 - add different default layout for autoplay prompt.

https://reviewboard.mozilla.org/r/259832/#review267292

I see Johann suggested using a WebExtension Experiment in comment 4, has this option been explored?

It should be possible to modify PermissionUI.AutoplayPermissionPrompt.prototype from a WebExtension Experiment.

If this turns out to be impossible or extremely difficult I would be OK with the approach in the current patch.

::: modules/libpref/init/all.js:570
(Diff revision 2)
>  
>  // Whether to autostart a media element with an |autoplay| attribute.
>  // ALLOWED=0, BLOCKED=1, PROMPT=2, defined in dom/media/Autoplay.idl
>  pref("media.autoplay.default", 0);
>  
> +// Use to control the default setting layout of doorhanger.

I would add "for a Shield study" at the end of this sentence.

::: modules/libpref/init/all.js:575
(Diff revision 2)
> +// Use to control the default setting layout of doorhanger.
> +// 0 = [Allow], [Remember this decision] unchecked
> +// 1 = [Don’t Allow], [Remember this decision] unchecked
> +// 2 = [Allow], [Remember this decision] checked
> +// 3 = [Don’t Allow], [Remember this decision] checked
> +pref("media.autoplay.prompt-layout", 2);

This pref is for code that lives in browser/ so it should be in browser/app/profile/firefox.js rather than in modules/libpref

Comment 5

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] from comment #4)
> Comment on attachment 8995377 [details]
> Bug 1478875 - add different default layout for autoplay prompt.
> 
> https://reviewboard.mozilla.org/r/259832/#review267292
> 
> I see Johann suggested using a WebExtension Experiment in comment 4

I meant bug 1475099 comment 4.

Comment 6

[Alastor Wu [:alwu]]

Thank for your suggestion!
This issue was fixed in my experiments code by overriding the methods in AutoplayPermissionPrompt's prototype.

Fixed here : https://github.com/alastor0325/autoplay-shield-study/commit/b53401a1fb5647a0157ac2371929533c417d6657