Bug 467035 Comment 16 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to :Gijs (he/him) from comment #15)

> (In reply to Alex Catarineu from comment #13)
> 
> > I checked, and these seem to come from `parseXULToFragment` calls done in UI like https://hg.mozilla.org/mozilla-central/file/c6d806b496845985516cc04342c04988aa1817dd/toolkit/content/widgets/wizard.js#l40. These end up calling `DomParser::ParseFromString` which sets the principal to null in the case of system principal. So the XML gets parsed and the DTDs are loaded with null principal, and therefore blocked after this patch.
> 
> I expect that our best bet is forcing the DOMParser in question to be using a different principal that passes the checks - perhaps a codebase principal for an about: URI that we know is allowed access and unprivileged (about:mozilla would be fitting, perhaps?). Either that or building some other special case. Brian/Christoph, thoughts?

Assuming Christoph thinks this it's OK to allow non-null principal for parseFromString in these cases, I will add that:
- For the customElements.js case this is only ever happening with system principal docs (injected here https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/components/processsingleton/CustomElementsListener.jsm#14). 
- We are also calling forceEnableXULXBL() on the DOMParser so perhaps that is a signal to allow for a non-null principal: https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/content/customElements.js#202-203. Or we could add a new option to allow it.

For videocontrols, perhaps we could do one of:
1) Expose a `[Func="IsChromeOrXBLOrUAWidget"]` method on DOMParser that's like parseFromStringNonNullPrincipal from videocontrols UA widget via: https://firefox-source-docs.mozilla.org/toolkit/content/toolkit_widgets/ua_widget.html#the-javascript-sandbox.
2) As mentioned above, figuring out how to convert this to Fluent may be a better solution - I think it was done with DTD just because it was the closest available option.
3) Store the strings in a properties file, read them from chrome code and pass them into the widget constructor similar to how we are now passing prefs for picture in picture: (https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/actors/UAWidgetsChild.jsm#99). Then take those strings as variables into the widget and set the textContent, etc appropriately instead of from the DTD. If you wanted to test that out without moving strings into a properties file, you could use DOMParser from UAWidgetsChild.jsm, create a document fragment that includes those entities into nodes, then read the textContent from the nodes and pass them into the widget constructor that way.
(In reply to :Gijs (he/him) from comment #15)

> (In reply to Alex Catarineu from comment #13)
> 
> > I checked, and these seem to come from `parseXULToFragment` calls done in UI like https://hg.mozilla.org/mozilla-central/file/c6d806b496845985516cc04342c04988aa1817dd/toolkit/content/widgets/wizard.js#l40. These end up calling `DomParser::ParseFromString` which sets the principal to null in the case of system principal. So the XML gets parsed and the DTDs are loaded with null principal, and therefore blocked after this patch.
> 
> I expect that our best bet is forcing the DOMParser in question to be using a different principal that passes the checks - perhaps a codebase principal for an about: URI that we know is allowed access and unprivileged (about:mozilla would be fitting, perhaps?). Either that or building some other special case. Brian/Christoph, thoughts?

Assuming Christoph thinks this it's OK to allow non-null principal for parseFromString in these cases, I will add that:
- For the customElements.js case this is only ever happening with system principal docs (injected here https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/components/processsingleton/CustomElementsListener.jsm#14). 
- We are also calling forceEnableXULXBL() on the DOMParser so perhaps that is a signal to allow for a non-null principal: https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/content/customElements.js#202-203. Or we could add a new option to allow it.

For videocontrols, perhaps we could do one of:
1) Expose a `[Func="IsChromeOrXBLOrUAWidget"]` method on DOMParser called `parseFromStringNonNullPrincipal` or similar from videocontrols UA widget via: https://firefox-source-docs.mozilla.org/toolkit/content/toolkit_widgets/ua_widget.html#the-javascript-sandbox.
2) As mentioned above, figuring out how to convert this to Fluent may be a better solution - I think it was done with DTD just because it was the closest available option.
3) Store the strings in a properties file, read them from chrome code and pass them into the widget constructor similar to how we are now passing prefs for picture in picture: (https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/toolkit/actors/UAWidgetsChild.jsm#99). Then take those strings as variables into the widget and set the textContent, etc appropriately instead of from the DTD. If you wanted to test that out without moving strings into a properties file, you could use DOMParser from UAWidgetsChild.jsm, create a document fragment that includes those entities into nodes, then read the textContent from the nodes and pass them into the widget constructor that way.

Back to Bug 467035 Comment 16