Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Enable Web Components by default for certified apps

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: overholt, Assigned: wchen)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Trunk
mozilla33
dev-doc-needed
Points:
---

Firefox Tracking Flags

(firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v1.3 unaffected, b2g-v1.3T unaffected, b2g-v1.4 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
This will let some gaia developers and others more easily give feedback.  William is hoping to do this before the end of June 2014.
I'd like to see this also on the Aurora/Nightly channels for desktop, rather than using a polyfill.
(Reporter)

Comment 2

3 years ago
(In reply to Kohei Yoshino [:kohei] from comment #1)
> I'd like to see this also on the Aurora/Nightly channels for desktop, rather
> than using a polyfill.

Last I heard, there were still pending spec changes so we shouldn't put it on pre-release (or any) channels without a pref until that is sorted out.
(Assignee)

Comment 3

3 years ago
Created attachment 8448889 [details] [diff] [review]
Enable web components for certified apps.

Gaia is using web components by turning on the flag in B2G, but this enables web components globally and essentially exposes it to the web on that platform. We need to change all the web components API checks to also check for certified apps, remove the flag on B2G and make sure these changes gets uplifted.
Attachment #8448889 - Flags: review?(gkrizsanits)
Attachment #8448889 - Flags: review?(bugs)

Comment 4

3 years ago
Comment on attachment 8448889 [details] [diff] [review]
Enable web components for certified apps.

> static uint32_t ToLinkMask(const nsAString& aLink)
> { 
>   if (aLink.EqualsLiteral("prefetch"))
>     return nsStyleLinkElement::ePREFETCH;
>   else if (aLink.EqualsLiteral("dns-prefetch"))
>     return nsStyleLinkElement::eDNS_PREFETCH;
>   else if (aLink.EqualsLiteral("stylesheet"))
>     return nsStyleLinkElement::eSTYLESHEET;
>   else if (aLink.EqualsLiteral("next"))
>     return nsStyleLinkElement::eNEXT;
>   else if (aLink.EqualsLiteral("alternate"))
>     return nsStyleLinkElement::eALTERNATE;
>-  else if (aLink.EqualsLiteral("import") && nsStyleLinkElement::IsImportEnabled())
>+  else if (aLink.EqualsLiteral("import"))
This feels a bit error prone. Couldn't you keep IsImportEnabled and require one to pass nsIPrincipal.
and then
else if (aLink.EqualsLiteral("import") && aPrincipal && nsStyleLinkElement::IsImportEnabled(aPrincipal))


>+      } else if (linkTypes & eHTMLIMPORT &&
>+                 nsStyleLinkElement::IsImportEnabled(NodePrincipal())) {
Then you wouldn't need this
Attachment #8448889 - Flags: review?(bugs) → review-
(Assignee)

Comment 5

3 years ago
Created attachment 8449057 [details] [diff] [review]
Enable web components for certified apps. v2

(In reply to Olli Pettay [:smaug] from comment #4)
> Comment on attachment 8448889 [details] [diff] [review]
> Enable web components for certified apps.
> 
> This feels a bit error prone. Couldn't you keep IsImportEnabled and require
> one to pass nsIPrincipal.
> and then
> else if (aLink.EqualsLiteral("import") && aPrincipal &&
> nsStyleLinkElement::IsImportEnabled(aPrincipal))
ToLinkMask, and its only consumer (ParseLinkTypes) are pretty simple static functions that map strings to enums and it doesn't seem worth it to require that the callers provide a principal when there are only two callers that care about an eHTMLIMPORT return value (both of which already end up checking IsImportEnabled()). It seems to me that the IsImportEnabled() check in ToLinkMask wasn't necessary in the first place. I understand your concern about this being a little error prone, but I think that any behaviour that needs to be gated by IsImportEnabled() is better done with an explicit check rather than checking eHTMLIMPORT on the link mask.

I've created a new patch with your review comments addressed. I've left the old patch up in case you've changed your mind.
Attachment #8449057 - Flags: review?(gkrizsanits)
Attachment #8449057 - Flags: review?(bugs)

Updated

3 years ago
Attachment #8449057 - Flags: review?(bugs) → review+
Attachment #8448889 - Attachment is obsolete: true
Attachment #8448889 - Flags: review?(gkrizsanits)
Attachment #8449057 - Flags: review?(gkrizsanits) → review+
(Assignee)

Comment 6

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6c82c9d6b8c
https://hg.mozilla.org/mozilla-central/rev/b6c82c9d6b8c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Assignee)

Comment 8

3 years ago
Comment on attachment 8449057 [details] [diff] [review]
Enable web components for certified apps. v2

Gaia 2.0 currently has web component features turned on via a preference, this has a consequence of enabling the APIs for all of the web, but these APIs are experimental and intended only for certified apps right now. This patch enable web components by default for certified apps and will let us remove the preference from Gaia.

Approval Request Comment
[Feature/regressing bug #]: Bug 1006118
[User impact if declined]: Users will be exposed to DOM API that is not ready to be exposed to the general web.
[Describe test coverage new/current, TBPL]: There are many web component tests running on TBPL.
[Risks and why]: Low risk, this patch enables web component features for certified apps that are already being used in Gaia.
[String/UUID change made/needed]: None
Attachment #8449057 - Flags: approval-mozilla-aurora?
status-b2g-v1.3: --- → unaffected
status-b2g-v1.3T: --- → unaffected
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
Comment on attachment 8449057 [details] [diff] [review]
Enable web components for certified apps. v2

Aurora+ 

:overholt - Glad to see we're asking "Is this good for the WEB?"
Attachment #8449057 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/936ff3b9a5ce
status-b2g-v2.0: affected → fixed
status-firefox31: --- → wontfix
status-firefox32: --- → fixed
status-firefox33: --- → fixed
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.