1146938 - Prepare backout patch for screensharing in Fx38

Status: VERIFIED FIXED

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

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: enable Loop screensharing based on a pref

Comment 2

[Mike de Boer [:mikedeboer]]

(patch based on m-a)

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8583107 [details] [diff] [review]
Patch v1: enable Loop screensharing based on a pref

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

This breaks the desktop unit tests. I'd also prefer navigator.mozLoop to be passed in like we do elsewhere.

I think this patch should effectively be backing out bug 1126289 but making sure the pref is now true by default.

Comment 4

[Mike de Boer [:mikedeboer]]

(In reply to Mark Banner (:standard8) from comment #3)
> This breaks the desktop unit tests. I'd also prefer navigator.mozLoop to be
> passed in like we do elsewhere.
> 
> I think this patch should effectively be backing out bug 1126289 but making
> sure the pref is now true by default.

You're absolutely right, apologies for the noise. Re-inventing-the-wheel-syndrome *sigh*

Comment 5

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: enable Loop screensharing based on a pref

This patch is based on latest fx-team, instead.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8583778 [details] [diff] [review]
Patch v2: enable Loop screensharing based on a pref

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

That's better, thanks.

Comment 7

[Mike de Boer [:mikedeboer]]

Attachment: Aurora-patch

Rebased patch for Aurora, carrying over r=Standard8.

Approval Request Comment
[Feature/regressing bug #]: Loop screen-/ tab-sharing feature
[User impact if declined]: This patch allows us to disable the screensharing feature by setting the pref `loop.screensharing.enabled` to `false`. This will come in handy when the release of this feature will be postponed to Fx 38.1
[Describe test coverage new/current, TreeHerder]: Aurora-only patch. Revert of bug 1126289, before which this pref existed and yielded green runs.
[Risks and why]: minor.
[String/UUID change made/needed]: n/a.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

FYI, there is a define (EARLY_BETA_OR_EARLIER) to disable a feature at the middle of the beta cycle
https://wiki.mozilla.org/Platform/Channel-specific_build_defines#EARLY_BETA_OR_EARLIER

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> FYI, there is a define (EARLY_BETA_OR_EARLIER) to disable a feature at the
> middle of the beta cycle
> https://wiki.mozilla.org/Platform/Channel-
> specific_build_defines#EARLY_BETA_OR_EARLIER

Thanks, Sylvestre. Our intention is not to disable it unless absolutely necessary. This depends on the release strategy for Reader-mode.

Comment 10

[Mike de Boer [:mikedeboer]]

Pushed to Aurora as: https://hg.mozilla.org/releases/mozilla-aurora/rev/bce8a0df6baf

Comment 11

[Bogdan Maris, Desktop QA]

Verified on Windows 8.1 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit using Firefox 38 beta 1 that toggleing pref 'loop.screenshare.enabled' will turn on or off Hello screensharing feature.