1038439 - Remove enabling of web components preference in Gaia

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[William Chen [:wchen]]

Web components are currently turned on in Gaia via a preference. This should not be the case because it enables the feature for the general web and it is currently intended only as an experimental feature for certified apps.

It looks like it was a temporary measure to develop with web components in certified apps until bug 1000199 had landed. Now that web components are enabled for certified apps by default the pref needs to go away and the change needs to be uplifted to v2.0

Comment 1

[William Chen [:wchen]]

Attachment: pull request

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8455734 [details] [review]
pull request

Thanks.

Comment 3

[William Chen [:wchen]]

It looks like it's not going to be so easy to get rid of that preference in Gaia. We have a pretty big problem. After removing the pref, there are tests failing in the unit tests because it looks like they are not running with certified app privilege. There are a few failures in collections, camera and search but most are in sharedtest which does not even have certified set in its manifest.

We really need to get rid of that pref to make sure that developers are not using web components in non-certified apps and hopefully nobody is currently using web components where they should not be using it. We REALLY need to make sure that we don't end up shipping it to the web. This needs to be fixed soon and in a way that we can uplift to v2.0.

There are a few ways we could solve this problem but I'm not sure if we currently have the capabilities to do these:
a) Only enable the preference while running tests.
b) Run tests with certified privilege.
c) Disable the failing tests.
d) ?

Vivien: Do you know if we can do any of the things above? I'm not familiar with the Gaia testing infrastructure so it would be great if you could point me towards someone who can help me fix this issue.

Comment 4

[James Lal [:lightsofapollo]]

Talked with @wchen on IRC:

We can keep the flag (for testings) but obviously only enable this as needed for things like unit tests in firefox (which have their own set of preferences now). 

That aside there must be some (probably significant) value to keeping the pref for at minimum internal testing using firefox nightly and probably for third party devs who are already working on WC things for chrome.

Comment 5

[James Lal [:lightsofapollo]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/22134

Comment 6

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8462365 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/22134

looks good and r=yurenju, but please use single quote for debug-prefs.js and add /* global pref */ in it.

Comment 7

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/4f77cd1da2a20761673e1c159507f6720890b2cb

Comment 8

[James Lal [:lightsofapollo]]

Comment on attachment 8462365 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/22134

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: We expose incomplete/crashy web components implementation to all web content.
[Testing completed]: CI/Manual
[Risk to taking this patch] (and alternatives if risky): None, any issues should be seen immediately if there was some...
[String changes made]:

Comment 9

[Julien Wajsberg [:julienw]]

You didn't update the build tests so travis is red. I have no clue why TBPL is green, does it run this test?

https://travis-ci.org/mozilla-b2g/gaia/jobs/31306371

Comment 10

[Julien Wajsberg [:julienw]]

backing out: 9e5bc4918eec60ca1ef893c5f63d38788a4e9549

Comment 11

[bhavana bajaj [:bajaj]]

Comment on attachment 8462365 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/22134

clearing the approval request given this is not resolved yet on master/central.

Comment 12

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/63422178f24a6fede5124c3f9184c0fc9f9c8a6b

Comment 13

[James Lal [:lightsofapollo]]

Comment on attachment 8462365 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/22134

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 14

[Andrew Overholt [:overholt]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1052150 adds some fuel to the "uplift this" fire in case it's needed :)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/6155d46822edc6f799141caa8cf86209fb623c34