1197401 - Remove our HTML import implementation

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

AFAIK we have decided that we're not going to ship HTML imports, but we still have the code in the tree that people need to keep working.  If that decision is final, it would be nice to delete the code so that we don't have to keep working on it...

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Considering it can be resurrected from hg if necessary, this would be nice.

I'm running into issues with import over in bug 1184607.  It seems html imports has the wrong nsContentPolicyType which will likely confuse our service worker code if it the two interact.

Comment 2

[Anne (:annevk)]

That decision is final. If we do HTML imports again it will likely have a different shape from what it has today.

Comment 3

[(no longer active)]

Of course, gaia uses imports.  :(

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #3)
> Of course, gaia uses imports.  :(

Are you sure? Last time I checked (it was a while ago) they used a JS implemented custom implementation instead the native one this bug is about.

Plus imports are hidden behind a flag that is off by default (tests relying on it turns that flag on), so we could just turn off the tests as a start, and stop worrying about breaking them if this blocking people.

Comment 5

[(no longer active)]

I'm relatively sure.  See <http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp#5879> and then <https://mxr.mozilla.org/gaia/search?string=moz-extremely-unstable-and-will-change-webcomponents>.

Comment 6

[Gabor Krizsanits (INACTIVE)]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #5)
> I'm relatively sure.  See
> <http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp#5879>
> and then
> <https://mxr.mozilla.org/gaia/search?string=moz-extremely-unstable-and-will-
> change-webcomponents>.

I'm not sure what that flag does, but imports are relying on this one: http://mxr.mozilla.org/mozilla-central/source/dom/base/nsStyleLinkElement.cpp#139 and I'm pretty sure it is only turned on for tests.

Comment 7

[(no longer active)]

You're right, good!

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

I'm going to take this.  With html imports enabled in tests we're actually hitting assertions in a service worker WPT.  We also have other intermittent test failures in the imports tests.  Lets finally de-orbit this to reduce our maintenance load.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Remove the disabled HTML imports implementation. r=wchen

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2ca56c4967f11e0e1b042672eece01581067e71

Comment 12

[Chris Mills [:cmills]]

Setting straight to dev-doc-complete; I went to the HTML Imports page, and simple removed the mention of being able to turn HMTL Imports on in Fx using the pref.

https://developer.mozilla.org/en-US/docs/Web/Web_Components/HTML_Imports

Since we never shipped this, I'm not going to mention it in the Fx rel notes.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Remove the disabled HTML imports implementation. r=wchen

William, this patch attempts to remove our pref'ed off HTML imports implementation.

I basically did this by removing import specific stuff in HTMLLinkElement/nsDocument/ImportManager and then fixing build errors.  I also removed the pref in the test profile and the mochitests themselves.  I updated test expectations for WPT.

Comment 14

[William Chen [:wchen]]

Comment on attachment 8875328 [details] [diff] [review]
Remove the disabled HTML imports implementation. r=wchen

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

::: dom/base/nsIDocument.h
@@ -2854,5 @@
> -  virtual bool HasSubImportLink(nsINode* aLink) = 0;
> -  virtual uint32_t IndexOfSubImportLink(nsINode* aLink) = 0;
> -  virtual void AddSubImportLink(nsINode* aLink) = 0;
> -  virtual nsINode* GetSubImportLink(uint32_t aIdx) = 0;
> -

We should also get rid of the forward declaration. https://dxr.mozilla.org/mozilla-central/rev/e61060be36424240058f8bef4c5597f401bc8b7e/dom/base/nsIDocument.h#144

::: dom/html/nsHTMLDocument.cpp
@@ -1893,5 @@
>    mTooDeepWriteRecursion =
>      (mWriteLevel > NS_MAX_DOCUMENT_WRITE_DEPTH || mTooDeepWriteRecursion);
>    NS_ENSURE_STATE(!mTooDeepWriteRecursion);
>  
> -  if (!IsHTMLDocument() || mDisableDocWrite || !IsMasterDocument()) {

We also need to get rid of a check for import here: https://dxr.mozilla.org/mozilla-central/rev/e61060be36424240058f8bef4c5597f401bc8b7e/dom/html/nsHTMLDocument.cpp#534

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Remove the disabled HTML imports implementation. r=wchen

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c41595a24fbd
Remove the disabled HTML imports implementation. r=wchen

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c41595a24fbd