Closed Bug 1279218 Opened 4 years ago Closed 2 years ago

Remove <applet> element

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: annevk, Assigned: qdot)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: Blocked on removing Java support. Revisit after Firefox 52 ESR.)

Attachments

(7 files)

https://github.com/whatwg/html/pull/1399 has a good summary of what that entails. Chrome has removed support already and Edge is interested.

https://bugs.webkit.org/show_bug.cgi?id=157926 is the corresponding WebKit issue.
Benjamin, I think you're tracking this.
Flags: needinfo?(benjamin)
Whiteboard: btpp-followup-2016-07-05
Once we drop support for Java in FF52, I'm all about this. Before then I'm not willing to take any risks.
Depends on: npapi-eol
Flags: needinfo?(benjamin)
Priority: -- → P3
Whiteboard: btpp-followup-2016-07-05 → Blocked on removing Java support. Revisit for FF52.
Whiteboard: Blocked on removing Java support. Revisit for FF52. → Blocked on removing Java support. Revisit for FF52. [btpp-backlog]
Please wait until 53 for this, because we're still supporting applet via a pref on 52 ESR.
Target Milestone: --- → mozilla53
Flags: needinfo?(amarchesini)
qdot, do you want to take it?
Flags: needinfo?(amarchesini) → needinfo?(kyle)
Assignee: nobody → kyle
Flags: needinfo?(kyle)
Whiteboard: Blocked on removing Java support. Revisit for FF52. [btpp-backlog] → Blocked on removing Java support. Revisit after Firefox 52 ESR.
Why is ESR relevant to mainline development?
We had to release the 52 ESR with applet/java support before starting on this bug.
Duplicate of this bug: 1351814
Duplicate of this bug: 1351886
Adding webplatform test cleanup to the landing criteria for this bug, since landing this is basically the trigger for html spec removal also. See https://github.com/whatwg/html/pull/1399#issuecomment-303875482
Comment on attachment 8886392 [details]
Bug 1279218 - Remove Applet/Java support from nsObjectLoadingContent

https://reviewboard.mozilla.org/r/155862/#review162908

r=me either way but please file followups for additional cleanup if appropriate.

::: dom/base/nsObjectLoadingContent.h:304
(Diff revision 1)
>        eSupportImages       = 1u << 0, // Images are supported (imgILoader)
>        eSupportPlugins      = 1u << 1, // Plugins are supported (nsIPluginHost)
>        eSupportDocuments    = 1u << 2, // Documents are supported
>                                          // (nsIDocumentLoaderFactory)
>                                          // This flag always includes SVG
> -      eSupportClassID      = 1u << 3, // The classid attribute is supported
> +      eSupportClassID      = 1u << 3, // The classid attribute is supported. No

Will there be a followup to remove eSupportClassID completely?

::: dom/base/nsObjectLoadingContent.h:424
(Diff revision 1)
>       * - mOriginalURI         : The src or data attribute on the element
>       * - mURI                 : The final URI, considering mChannel if
>       *                          mChannelLoaded is set
>       * - mContentType         : The final content type, considering mChannel if
>       *                          mChannelLoaded is set
>       * - mBaseURI             : The object's base URI, which may be set by the

Is `codebase` the only way to get a custom base URI for this? If so do we still need `mBaseURI` at all?
Attachment #8886392 - Flags: review?(benjamin) → review+
Comment on attachment 8886394 [details]
Bug 1279218 - Remove tests related to the applet tag;

https://reviewboard.mozilla.org/r/155866/#review162914

Does it make sense to/can you remove all of testplugin/javaplugin as part of this?
Comment on attachment 8886393 [details]
Bug 1279218 - Remove Applet element support from WebBrowserPersist;

https://reviewboard.mozilla.org/r/155864/#review163002
Attachment #8886393 - Flags: review?(jld) → review+
Comment on attachment 8886391 [details]
Bug 1279218 - Remove Applet tag;

https://reviewboard.mozilla.org/r/155860/#review163836

This needs changes to test_interfaces.html, I would think.

::: dom/bindings/Bindings.conf:405
(Diff revision 2)
>  'History': {
>      'headerFile': 'nsHistory.h',
>      'nativeType': 'nsHistory'
>  },
>  
>  'HTMLAppletElement': {

Why is this needed at all?  We're removing this interface, aren't we?  So what's the point of this descriptor?

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
>  
>    NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED_NO_UNLINK(HTMLSharedObjectElement,
>                                                       nsGenericHTMLElement)
>  
> -  // WebIDL API for <applet>
> +  // WebIDL <embed> api
> -  void GetAlign(DOMString& aValue)

Why are you losing the GetAlign implementation?  Please don't.

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
> -  // XPCOM GetCodebase is ok; note that it's a URI attribute
> -  void SetCodeBase(const nsAString& aValue, ErrorResult& aRv)
> -  {
> -    SetHTMLAttr(nsGkAtoms::codebase, aValue, aRv);
> -  }
> -  void GetHeight(DOMString& aValue)

Please don't lose GetHeight either.

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
> -  }
> -  void SetHspace(uint32_t aValue, ErrorResult& aRv)
> -  {
> -    SetUnsignedIntAttr(nsGkAtoms::hspace, aValue, 0, aRv);
> -  }
> -  void GetName(DOMString& aValue)

Or GetName.

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
> -  }
> -  void SetVspace(uint32_t aValue, ErrorResult& aRv)
> -  {
> -    SetUnsignedIntAttr(nsGkAtoms::vspace, aValue, 0, aRv);
> -  }
> -  void GetWidth(DOMString& aValue)

Or GetWidth.

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
>    {
>      SetHTMLAttr(nsGkAtoms::width, aValue, aRv);
>    }
> -
> -  // WebIDL <embed> api
> -  // XPCOM GetSrc is ok; note that it's a URI attribute

Please don't lose this comment.

::: dom/html/HTMLSharedObjectElement.h
(Diff revision 2)
> -  // XPCOM GetSrc is ok; note that it's a URI attribute
>    void SetSrc(const nsAString& aValue, ErrorResult& aRv)
>    {
>      SetHTMLAttr(nsGkAtoms::src, aValue, aRv);
>    }
> -  void GetType(DOMString& aValue)

And this function should stay too.

::: dom/html/HTMLSharedObjectElement.h:127
(Diff revision 2)
>                                            bool aNotify) override;
>  
>  private:
> -  virtual ~HTMLSharedObjectElement();
> +  ~HTMLSharedObjectElement();
>  
>    nsIAtom *URIAttrName() const

Does this function still need to exist?  It only has one consumer; that consumer could compare to nsGkAtoms::src directly.

::: dom/html/HTMLSharedObjectElement.cpp:93
(Diff revision 2)
>                                 nsIImageLoadingContent,
>                                 imgIOnloadBlocker,
>                                 nsIChannelEventSink)
>    NS_INTERFACE_TABLE_TO_MAP_SEGUE
> -  NS_INTERFACE_MAP_ENTRY_IF_TAG(nsIDOMHTMLAppletElement, applet)
>    NS_INTERFACE_MAP_ENTRY_IF_TAG(nsIDOMHTMLEmbedElement, embed)

Does this still need to be IF_TAG?  I don't think it does.

::: dom/html/HTMLSharedObjectElement.cpp:401
(Diff revision 2)
>      MOZ_ASSERT(mNodeInfo->Equals(nsGkAtoms::embed));
>      return nsIContentPolicy::TYPE_INTERNAL_EMBED;
>    }
>  }
>  
> +NS_IMPL_STRING_ATTR(HTMLSharedObjectElement, Align, align)

If we're touching this stuff anyway, might be better to remove it altogether (from here and nsIDOMHTMLEmbedElement.idl).  Followup better for that.
Attachment #8886391 - Flags: review?(bzbarsky) → review+
Comment on attachment 8886391 [details]
Bug 1279218 - Remove Applet tag;

https://reviewboard.mozilla.org/r/155860/#review163842

::: dom/html/HTMLObjectElement.cpp:563
(Diff revision 2)
>  }
>  
>  uint32_t
>  HTMLObjectElement::GetCapabilities() const
>  {
> -  return nsObjectLoadingContent::GetCapabilities() | eSupportClassID;
> +  return nsObjectLoadingContent::GetCapabilities();

Oh, shouldn't this be in part 2?  Seems to me like it should be.
Comment on attachment 8886392 [details]
Bug 1279218 - Remove Applet/Java support from nsObjectLoadingContent

https://reviewboard.mozilla.org/r/155862/#review163848

::: dom/base/nsObjectLoadingContent.cpp
(Diff revision 2)
>  #include "nsIDocShellLoadInfo.h"
>  #include "nsIDocument.h"
>  #include "nsIDOMCustomEvent.h"
>  #include "nsIDOMDocument.h"
>  #include "nsIDOMHTMLObjectElement.h"
> -#include "nsIDOMHTMLAppletElement.h"

This should have been in part 1.

::: dom/base/nsObjectLoadingContent.cpp:892
(Diff revision 2)
>      if (name.IsEmpty())
>        continue;
>  
>      nsCOMPtr<nsIContent> parent = element->GetParent();
>      nsCOMPtr<nsIDOMHTMLObjectElement> domObject;
> -    nsCOMPtr<nsIDOMHTMLAppletElement> domApplet;
> +    while (!domObject && parent) {

Followup bug to rewrite this in terms of FromContent?

::: dom/base/nsObjectLoadingContent.cpp
(Diff revision 2)
> -    // Java expects a directory as the codebase, or else it will construct
> -    // relative URIs incorrectly :(
> -    codebaseStr.Assign('.');
> -  }
> -
> -  if (!codebaseStr.IsEmpty()) {

Hmm.  So we used to consider the "codebase" attr on non-applet tags too here?  I think we did...  Is changing that ok?

::: dom/base/nsObjectLoadingContent.cpp
(Diff revision 2)
> -  /// Check if the original (pre-channel) content-type or URI changed, and
> -  /// record mOriginal{ContentType,URI}

Why is this comment being removed?

::: dom/plugins/base/nsPluginInstanceOwner.cpp
(Diff revision 2)
>  #include "nsIPluginWidget.h"
>  #include "nsViewManager.h"
>  #include "nsIDocShellTreeOwner.h"
>  #include "nsIDOMHTMLObjectElement.h"
>  #include "nsIAppShell.h"
> -#include "nsIDOMHTMLAppletElement.h"

This sould have been in part 1, right?

::: dom/plugins/base/nsPluginInstanceOwner.cpp:1307
(Diff revision 2)
>    NS_ENSURE_ARG_POINTER(result);
>  
>    *result = nsPluginTagType_Unknown;
>  
>    nsCOMPtr<nsIContent> content = do_QueryReferent(mContent);
> -  if (content->IsHTMLElement(nsGkAtoms::applet))
> +  if (content->IsHTMLElement(nsGkAtoms::embed))

This should have been in part 1, I would think.
Attachment #8886392 - Flags: review?(bzbarsky) → review+
Comment on attachment 8886394 [details]
Bug 1279218 - Remove tests related to the applet tag;

https://reviewboard.mozilla.org/r/155866/#review163854
Attachment #8886394 - Flags: review?(bzbarsky) → review+
Comment on attachment 8886395 [details]
Bug 1279218 - Additional applet tag logic removal;

https://reviewboard.mozilla.org/r/155868/#review163856

Should presumably be folded into part 1 before landing?
Attachment #8886395 - Flags: review?(bzbarsky) → review+
OK, so changes that should presumably be happening (based on code search and <https://github.com/whatwg/html/pull/1399#issue-158960327>) that I'm not seeing:

1) Remove applet from nsGenericHTMLElement::ShouldExposeIdAsHTMLDocumentProperty.
2) Remove applet from nsGenericHTMLElement::CanHaveName.
3) Remove applet rom the isXULOrPluginElement bit in Element::GetBindingURL.
4) Remove applet from the special-check in EventStateManager::FireContextClick
5) Remove applet from IsAllNamedElement in HTMLAllCollection.cpp.
6) Changing document.applets to return an empty collection
7) Remove checks for applet in nsXMLContentSink::CloseElement and nsXMLContentSink::IsMonolithicContainer.
8) Remove check for applet in txMozillaXMLOutput::endElement
9) Remove dead checks for applet in nsPluginFrame::GetMinISize and nsPluginFrame::GetDesiredSize.
10) Remove special cases from HTML5 parser for applet (?)  Not sure which things should go and which
    should stay, here.  I don't know whether the HTML5 parser has something like the "monolithic
    container" bits from the XML parser above, for example.
11) Remove the DoneAddingChildren bit for applet in nsHtml5TreeBuilder::elementPopped
12) Rename HTMLSharedObjectElement to HTMLEmbedElement.
Flags: needinfo?(kyle)
Comment on attachment 8887670 [details]
Bug 1279218 - Remove Java Test Plugin and rest of Java references;

https://reviewboard.mozilla.org/r/158552/#review164352
Attachment #8887670 - Flags: review?(benjamin) → review+
Regarding point 10 in comment 31, the HTML standard continues to parse applet element start and end tags the same way as those of marquee and object. Anything that's not related to building a tree can go though (and was never in the standard to begin with).
Right, I'm not talking about changing the parsing model.  I'm talking about gecko-internal special handling like preventing parser interrupts inside <applet>, assuming we do that right now.
:wchen, I'm not sure where parser interrupts happen for tags as mentioned in Comment 34, and the parsing for applet looks like it basically follows marquee right now. Do you know if there's else I need to remove in the parser?
Flags: needinfo?(kyle) → needinfo?(wchen)
:bz, what would you recommend for created the null content list in nsHTMLDocument::Applets()? We don't have an nsContentList constructor that can just produce a null list on its own at the moment, and I'm not seeing any straight-forward way to create an nsIHTMLCollection that's not /actually/ a collection of something.
Flags: needinfo?(bzbarsky)
The HTML5 parser doesn't have any special handing when we create an applet element, the only extra thing we do is call DoneAddingChildren on the element when it gets popped off the stack in tree builder.
Flags: needinfo?(wchen)
We don't really have a thing for the "nothing in it" content list right now.

You could either do nsContentList with a nsContentListMatchFunc that always returns false, or create a new small subclass of nsBaseContentList that inherits from nsIHTMLCollection, implements nsIDOMHTMLCollection, and returns null and 0 as needed from the various methods involved.
Flags: needinfo?(bzbarsky)
Comment on attachment 8891478 [details]
Bug 1279218 - Rename HTMLSharedObjectElement to HTMLEmbedElement;

https://reviewboard.mozilla.org/r/162616/#review167954

It looks like this change just does a delete and add instead of "hg mv".  Please use "hg mv", which will not only preserve history but make for a much more reviewable diff...

::: dom/base/nsObjectLoadingContent.cpp:2960
(Diff revision 1)
>          if (child->IsHTMLElement(nsGkAtoms::embed)) {
> -          HTMLSharedObjectElement* embed = static_cast<HTMLSharedObjectElement*>(child);
> +          HTMLEmbedElement* embed = static_cast<HTMLEmbedElement*>(child);

This can become:

    if (auto* embed = HTMLEmbedElement::FromContent(child)) {

::: dom/bindings/Bindings.conf:427
(Diff revision 1)
>  'HTMLEmbedElement': {
> -    'nativeType': 'mozilla::dom::HTMLSharedObjectElement'
> +    'nativeType': 'mozilla::dom::HTMLEmbedElement'
>  },

This entire block can go away, since it now has the right name.
Attachment #8891478 - Flags: review?(bzbarsky) → review-
Even though I pushed https://reviewboard.mozilla.org/r/155868/diff/4#index_header this morning, it's marked as r+ from a week ago, before the patch even existed. It hasn't actually received a review yet and still needs to be looked at.
Flags: needinfo?(bzbarsky)
Comment on attachment 8886395 [details]
Bug 1279218 - Additional applet tag logic removal;

https://reviewboard.mozilla.org/r/155868/#review167982

r- for the leak bit.  The rest looks good.

::: dom/base/nsContentList.h:182
(Diff revision 4)
> +
> +protected:
> +  virtual ~nsEmptyContentList() {}
> +private:
> +  // This has to be a strong reference, the root might go away before the list.
> +  nsCOMPtr<nsINode> mRoot;

This needs to be cycle-collected; otherwise it will leak.  Note that nsSimpleContentList cycle-collects stuff.

::: editor/libeditor/HTMLEditUtils.cpp:598
(Diff revision 4)
>    ELEM(abbr, true, true, GROUP_PHRASE, GROUP_INLINE_ELEMENT),
>    ELEM(acronym, true, true, GROUP_PHRASE, GROUP_INLINE_ELEMENT),
>    ELEM(address, true, true, GROUP_BLOCK,
>         GROUP_INLINE_ELEMENT | GROUP_P),
> +  // While applet is no longer a valid tag, removing it here breaks the editor
> +  // in a lot of ways. Just leave it.

Something more specific than "a lot of ways" (e.g. which tests fail or something) would be nice here in case someone does want to take a stab at removing this.

::: layout/generic/nsPluginFrame.cpp:382
(Diff revision 4)
>  nsPluginFrame::GetMinISize(gfxContext *aRenderingContext)
>  {
>    nscoord result = 0;
>  
>    if (!IsHidden(false)) {
> -    if (mContent->IsAnyOfHTMLElements(nsGkAtoms::applet,
> +    if (mContent->IsAnyOfHTMLElements(nsGkAtoms::embed)) {

IsHTMLElement(), not IsAnyOfHTMLElements(), since now we just have the one tag.

::: layout/generic/nsPluginFrame.cpp:444
(Diff revision 4)
>  
>    aMetrics.Width() = aReflowInput.ComputedWidth();
>    aMetrics.Height() = aReflowInput.ComputedHeight();
>  
> -  // for EMBED and APPLET, default to 240x200 for compatibility
> -  if (mContent->IsAnyOfHTMLElements(nsGkAtoms::applet,
> +  // for EMBED, default to 240x200 for compatibility
> +  if (mContent->IsAnyOfHTMLElements(nsGkAtoms::embed)) {

Again, IsHTMLElement.

::: netwerk/streamconv/converters/nsUnknownDecoder.cpp
(Diff revision 4)
>        MATCHES_TAG("base")     ||
>        MATCHES_TAG("style")    ||
>        MATCHES_TAG("div")      ||
>        MATCHES_TAG("p")        ||
>        MATCHES_TAG("font")     ||
> -      MATCHES_TAG("applet")   ||

I'm not sure we should change this part, honestly.  This is a heuristic sniffer; it doesn't care whether the content is _valid_; it cares whether the sender wanted it treated as HTML.

It probably doesn't matter too much either way, but I'd prefer this part happen in a separate bug for regression-tracking purposes.
Attachment #8886395 - Flags: review+ → review-
Comment on attachment 8886395 [details]
Bug 1279218 - Additional applet tag logic removal;

https://reviewboard.mozilla.org/r/155868/#review167982

> I'm not sure we should change this part, honestly.  This is a heuristic sniffer; it doesn't care whether the content is _valid_; it cares whether the sender wanted it treated as HTML.
> 
> It probably doesn't matter too much either way, but I'd prefer this part happen in a separate bug for regression-tracking purposes.

I just removed this change, instead of possibly causing a regression.
Comment on attachment 8886395 [details]
Bug 1279218 - Additional applet tag logic removal;

https://reviewboard.mozilla.org/r/155868/#review168064
Attachment #8886395 - Flags: review?(bzbarsky) → review+
Comment on attachment 8891478 [details]
Bug 1279218 - Rename HTMLSharedObjectElement to HTMLEmbedElement;

https://reviewboard.mozilla.org/r/162616/#review168066

r=me with the nits below.  Thank you; this diff was much easier to see changes in!

::: dom/base/nsObjectLoadingContent.cpp:2961
(Diff revision 4)
>            nsStyleUtil::IsSignificantChild(child, true, false)) {
>          aType = eFallbackAlternate;
>        }
>        if (thisIsObject) {
> -        if (child->IsHTMLElement(nsGkAtoms::embed)) {
> -          HTMLSharedObjectElement* embed = static_cast<HTMLSharedObjectElement*>(child);
> +        if (auto embed = HTMLEmbedElement::FromContent(child)) {
> +          static_cast<HTMLEmbedElement*>(embed)->StartObjectLoad(true, true);

You don't need the static_cast.  "embed" is already HTMLEmbedElement*.  Just do:

    embed->StartObjectLoad(true, true);

::: dom/bindings/BindingUtils.cpp:2312
(Diff revision 4)
>  
>    JS::Rooted<JSObject*> maybeObjLC(aCx, aObj);
>    nsObjectLoadingContent* htmlobject;
>    nsresult rv = UNWRAP_OBJECT(HTMLObjectElement, &maybeObjLC, htmlobject);
>    if (NS_FAILED(rv)) {
>      rv = UnwrapObject<prototypes::id::HTMLEmbedElement,

This can just become:

    rv = UNWRAP_OBJECT(HTMLEmbedElement, &maybeObjLC, htmlobject);
  
since now the interface name and class name match, so you don't have to specify them separately.

::: dom/html/HTMLEmbedElement.cpp:26
(Diff revision 4)
>  #include "mozilla/dom/Event.h"
>  #endif
>  #include "mozilla/dom/HTMLObjectElement.h"
>  
>  
> -NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(SharedObject)
> +NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(Embed)

This can now be NS_IMPL_NS_NEW_HTML_ELEMENT(Embed), since you no longer need aFromParser in the constructor.

::: dom/html/HTMLEmbedElement.cpp:31
(Diff revision 4)
> -NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(SharedObject)
> +NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(Embed)
>  
>  namespace mozilla {
>  namespace dom {
>  
> -HTMLSharedObjectElement::HTMLSharedObjectElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo,
> +HTMLEmbedElement::HTMLEmbedElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo,

And the constructor can drop the aFromParser arg.

::: dom/html/HTMLEmbedElement.cpp:198
(Diff revision 4)
> -HTMLSharedObjectElement::IsHTMLFocusable(bool aWithMouse,
> +HTMLEmbedElement::IsHTMLFocusable(bool aWithMouse,
>                                           bool *aIsFocusable,
>                                           int32_t *aTabIndex)
>  {
> -  if (mNodeInfo->Equals(nsGkAtoms::embed) || Type() == eType_Plugin) {
>      // Has plugin content: let the plugin decide what to do in terms of

This needs to actually be outdented, right?  Same for the rest of this function, up to the '}'.

::: dom/html/HTMLEmbedElement.cpp
(Diff revision 4)
> -  if (mNodeInfo->Equals(nsGkAtoms::embed)) {
>      return &MapAttributesIntoRuleExceptHidden;
>    }

Needs to be outdented.

::: dom/tests/mochitest/bugs/test_bug396843.html:20
(Diff revision 4)
>  </p>
>  <div id="content" style="display: none">
>    
>  </div>
>  <pre id="test">
> -<script class="testbody" type="text/javascript">
> +  <script class="testbody" type="text/javascript">

Not sure why all the indentation changes in this file...
Attachment #8891478 - Flags: review?(bzbarsky) → review+
Comment on attachment 8891478 [details]
Bug 1279218 - Rename HTMLSharedObjectElement to HTMLEmbedElement;

https://reviewboard.mozilla.org/r/162616/#review168066

> You don't need the static_cast.  "embed" is already HTMLEmbedElement*.  Just do:
> 
>     embed->StartObjectLoad(true, true);

For some reason I do seem to need to cast it, otherwise this happens in clang:

/share/code/mozbuild/mc-hg/dom/base/nsObjectLoadingContent.cpp:2961:18: error: no member named 'StartObjectLoad' in 'nsGenericHTMLElement'
           embed->StartObjectLoad(true, true);
           ~~~~~  ^

> And the constructor can drop the aFromParser arg.

What about the call to SetIsNetworkCreated in the constructor? I see that NS_IMPL_NS_NEW_HTML_ELEMENT just drops the boolean, do we assume it to always or never be from the parser?
> /share/code/mozbuild/mc-hg/dom/base/nsObjectLoadingContent.cpp:2961:18: error: no member named 'StartObjectLoad' in 'nsGenericHTMLElement'

Oh.  That's because HTMLEmbedElement doesn't actually declare FromContent, so you're getting the nsGenericHTMLElement one.  And then the static_cast is NOT safe: you could have any HTML element!

You want to throw:

  NS_IMPL_FROMCONTENT_HTML_WITH_TAG(HTMLEmbedElement, embed)

in the class decl of HTMLEmbedElement to make the FromContent thing work.

> What about the call to SetIsNetworkCreated in the constructor?

Ah, I missed that line.  Then you do need aFromParser, so just keep things as they are.
Flags: needinfo?(bzbarsky)
Comment on attachment 8891478 [details]
Bug 1279218 - Rename HTMLSharedObjectElement to HTMLEmbedElement;

https://reviewboard.mozilla.org/r/162616/#review168146
Pushed by kmachulis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e6f1f895449d
Remove Applet tag; r=bz
https://hg.mozilla.org/integration/autoland/rev/1d9ee33174d5
Remove Applet/Java support from nsObjectLoadingContent; r=bsmedberg,bz
https://hg.mozilla.org/integration/autoland/rev/e825f2ea34c1
Remove Applet element support from WebBrowserPersist; r=jld
https://hg.mozilla.org/integration/autoland/rev/91274528b1f8
Remove tests related to the applet tag; r=bz
https://hg.mozilla.org/integration/autoland/rev/893ac8dfa0a1
Remove Java Test Plugin and rest of Java references; r=bsmedberg
https://hg.mozilla.org/integration/autoland/rev/1c702ea52e96
Additional applet tag logic removal; r=bz
https://hg.mozilla.org/integration/autoland/rev/2ee0b5a0560a
Rename HTMLSharedObjectElement to HTMLEmbedElement; r=bz
Target Milestone: mozilla53 → ---
I've documented this; I have marked the element page as obsolete, and included details of when it was dropped/removal progress in other browsers:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/applet

I also added a note here to mention how the document.applets property now returns an empty HTMLCollection:

https://developer.mozilla.org/en-US/docs/Web/API/Document/applets

Last, I added a note to 
https://developer.mozilla.org/en-US/Firefox/Releases/56#Removals_from_the_web_platform

Let me know if that look OK. Thanks!
Depends on: 1386389
when this landed the total number of compiler warnings were reduced:
== Change summary for alert #8435 (as of July 30 2017 18:36 UTC) ==

Improvements:

  2%  compiler warnings summary linux64 pgo      1,550.08 -> 1,516.92

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8435
https://github.com/webcompat/web-bugs/issues/10722

It seems to cause some of sites flash video can't play based on this bugfix?
I don't think so. Chrome doesn't support <applet> either per comment 0 and the issue you point to suggests the site works in Chrome.
Sites could be sniffing and depending on <applet> in Firefox somehow but not in Chrome.

Or it's possible we removed more, or less, stuff than Chrome did...

It would be good to have some sort of minimized testcase for the problem from that webcompat issue.
No longer blocks: 1433221
Depends on: 1433221
The regression range contains bug 1433221, not this bug.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.