Remove all support for <applet> handling in Chrome.

Remove all support for <applet> handling in Chrome.

blink-dev intent to remove thread:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/E9LKKtcKbv8/cB7QmTlACgAJ

BUG=470301
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201659

[dcheng, 2015-08-19 18:44:16]

dcheng@chromium.org changed reviewers:
+ pdr@chromium.org

[dcheng, 2015-08-19 18:44:18]

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/dom/html/l...
File LayoutTests/dom/html/level2/html/HTMLDocument08.html (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/dom/html/l...
LayoutTests/dom/html/level2/html/HTMLDocument08.html:26: <OBJECT
TYPE="application/x-java-applet"></OBJECT>
Since this test exercises the behavior of document.applets (which can include
<object> tags that have applets), I opted to update the test so that we keep
testing document.applets.

The same comment applies to the xhtml version of this test.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/H...
File LayoutTests/fast/dom/HTMLDocument/document-special-properties.html (left):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/H...
LayoutTests/fast/dom/HTMLDocument/document-special-properties.html:5: the
document object. This lookup is supposed to include applet, embed,
Since <applet> will no longer be treated specifically in Blink, I updated this
test to remove lookups by name, etc. of <applet> tags.

Similar logic applies to the rest of the test changes that weren't removal only
changes.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
File LayoutTests/fast/dom/html-collections-named-getter.html (left):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
LayoutTests/fast/dom/html-collections-named-getter.html:95:
testFirstItemReturnsFirstMatch('document.applets', 0, ['applet', 'applet']);
Ditto: I updated this test to use an <object> tag containing an applet, since it
exercises the behavior of document.applets.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
File LayoutTests/fast/dom/htmlallcollection-enumerated-properties.html (left):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
LayoutTests/fast/dom/htmlallcollection-enumerated-properties.html:7: <applet
code="#" width="0" height="0" id="appletId1" name="appletName1"></applet>
Since this test exercises the enumeration of the window named properties, and we
no longer provide any built-in behavior for <applet>, I updated the test to
remove tests for <applet>-specific behavior and rebaselined the results.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
File LayoutTests/fast/dom/htmlcollection-non-html.html (left):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/fast/dom/h...
LayoutTests/fast/dom/htmlcollection-non-html.html:78:
testDocumentProperty("applets", "applet");
Test exercises document.applets, so I've updated the test.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests19-data-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests19-data-expected.txt:7: 59
I think adding a failing expectation here is the most correct thing to do, but
I'm not 100% sure.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/imported/w...
File
LayoutTests/imported/web-platform-tests/html/dom/documents/dom-tree-accessors/nameditem-03-expected.txt
(right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/imported/w...
LayoutTests/imported/web-platform-tests/html/dom/documents/dom-tree-accessors/nameditem-03-expected.txt:1:
This is a testharness.js-based test.
Since the imported tests are based on tests that are upstream, I checked in
failing expectations.

[pdr., 2015-08-20 07:21:23]

pdr@chromium.org changed reviewers:
+ cbiesinger@chromium.org, mstensho@opera.com

[pdr., 2015-08-20 07:21:24]

I'm a little overloaded on reviews at the moment.

@cbiesinger or @mstensho, would you be up for reviewing this?

[mstensho (USE GERRIT), 2015-08-20 09:45:56]

I think you should put a link to the intent thread on blink-dev in the commit
message.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests19-data-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests19-data-expected.txt:7: 59
On 2015/08/19 18:44:17, dcheng wrote:
> I think adding a failing expectation here is the most correct thing to do, but
> I'm not 100% sure.

This one?

#data
<!doctype html>applet<frameset>
#errors
#document
| <!DOCTYPE html>
| <html>
|   <head>
|   <body>
|     <applet>

I think you should just delete it from
LayoutTests/html5lib/resources/tests19.dat

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests19-write-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests19-write-expected.txt:115: Test 59 of
108 in ../resources/tests19.dat failed. Input:
Yeah, just remove that test from tests19.dat

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests7-data-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests7-data-expected.txt:5: Test 11 of 30 in
../resources/tests7.dat failed. Input:
Need to update or remove this test from tests7.dat. Keeping it may be
worthwhile, to see that the inner P isn't nested inside the outer P. But, then
again, I suspect that we already have such a test somewhere.

https://codereview.chromium.org/1291723004/diff/100001/Source/core/html/HTMLA...
File Source/core/html/HTMLAppletElement.h (left):

https://codereview.chromium.org/1291723004/diff/100001/Source/core/html/HTMLA...
Source/core/html/HTMLAppletElement.h:32: class HTMLAppletElement final : public
HTMLPlugInElement {
A comment in LayoutPart::widget() refers to this class. Please remove it.

[dcheng, 2015-08-21 06:53:33]

PS 6->9 addresses the review comments.
PS 9->10 trims a few more stray references to HTMLAppletElement.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests19-write-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests19-write-expected.txt:115: Test 59 of
108 in ../resources/tests19.dat failed. Input:
On 2015/08/20 at 09:45:56, mstensho wrote:
> Yeah, just remove that test from tests19.dat

Done.

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
File LayoutTests/html5lib/generated/run-tests7-data-expected.txt (right):

https://codereview.chromium.org/1291723004/diff/100001/LayoutTests/html5lib/g...
LayoutTests/html5lib/generated/run-tests7-data-expected.txt:5: Test 11 of 30 in
../resources/tests7.dat failed. Input:
On 2015/08/20 at 09:45:56, mstensho wrote:
> Need to update or remove this test from tests7.dat. Keeping it may be
worthwhile, to see that the inner P isn't nested inside the outer P. But, then
again, I suspect that we already have such a test somewhere.

I've updated this to use the <object> tag. By my reading, the behavior is
equivalent (the fallback content of <object> consists of the children of the
element)

https://codereview.chromium.org/1291723004/diff/100001/Source/core/html/HTMLA...
File Source/core/html/HTMLAppletElement.h (left):

https://codereview.chromium.org/1291723004/diff/100001/Source/core/html/HTMLA...
Source/core/html/HTMLAppletElement.h:32: class HTMLAppletElement final : public
HTMLPlugInElement {
On 2015/08/20 at 09:45:55, mstensho wrote:
> A comment in LayoutPart::widget() refers to this class. Please remove it.

Done.

[mstensho (USE GERRIT), 2015-08-21 08:47:09]

I think you should put a link to the intent thread on blink-dev in the commit
message.

[esprehn, 2015-08-21 09:52:37]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-08-21 09:52:38]

This is changing the html parser and means <applet> parses as an
HTMLUnknownElement now, won't render as replace content, and won't respect any
of the presentation attributes either.

That's going to cause needless page breakage, can we just render fallback
content and leave the element handling instead?

[dcheng, 2015-08-21 15:40:29]

On 2015/08/21 at 08:47:09, mstensho wrote:
> I think you should put a link to the intent thread on blink-dev in the commit
message.

Oops, done

[dcheng, 2015-08-21 15:43:17]

On 2015/08/21 at 09:52:38, esprehn wrote:
> This is changing the html parser and means <applet> parses as an
HTMLUnknownElement now, won't render as replace content, and won't respect any
of the presentation attributes either.
> 
> That's going to cause needless page breakage, can we just render fallback
content and leave the element handling instead?

As far as I'm aware, fallback content doesn't respect the presentation
attributes. I believe the current way we're handling the removal is consistent
with the original design for fallback content: browsers that didn't implement
support for the <applet> tag would just renders its children. This is consistent
with how we'll handle it once we remove all support.

[dcheng, 2015-08-31 23:56:03]

mstensho: ping!

[mstensho (USE GERRIT), 2015-09-01 08:06:02]

lgtm - @esprehn, are you happy with this now?

[esprehn, 2015-09-01 08:31:19]

On 2015/09/01 at 08:06:02, mstensho wrote:
> lgtm - @esprehn, are you happy with this now?

I still think this is excessive ignoring the presentational attributes, but
okay. :/

[esprehn, 2015-09-01 08:31:55]

On 2015/09/01 at 08:31:19, esprehn wrote:
> On 2015/09/01 at 08:06:02, mstensho wrote:
> > lgtm - @esprehn, are you happy with this now?
> 
> I still think this is excessive ignoring the presentational attributes, but
okay. :/

Also dcheng@'s comment in #11 is wrong, we _do_ respect presentational
attributes when doing fallback content.

[dcheng, 2015-09-01 08:46:55]

On 2015/09/01 at 08:31:55, esprehn wrote:
> On 2015/09/01 at 08:31:19, esprehn wrote:
> > On 2015/09/01 at 08:06:02, mstensho wrote:
> > > lgtm - @esprehn, are you happy with this now?
> > 
> > I still think this is excessive ignoring the presentational attributes, but
okay. :/
> 
> Also dcheng@'s comment in #11 is wrong, we _do_ respect presentational
attributes when doing fallback content.

Yes, the discussion got a bit forked between here and the blink-dev thread.
However, as far as I can tell, <applet> is treated as an inline element by
default: in my testing, I wasn't able to observe the rendering differences
without display: block, display: inline-block, or the like.

[dglazkov, 2015-09-01 15:27:24]

On 2015/09/01 at 08:46:55, dcheng wrote:
> On 2015/09/01 at 08:31:55, esprehn wrote:
> > On 2015/09/01 at 08:31:19, esprehn wrote:
> > > On 2015/09/01 at 08:06:02, mstensho wrote:
> > > > lgtm - @esprehn, are you happy with this now?
> > > 
> > > I still think this is excessive ignoring the presentational attributes,
but okay. :/
> > 
> > Also dcheng@'s comment in #11 is wrong, we _do_ respect presentational
attributes when doing fallback content.
> 
> Yes, the discussion got a bit forked between here and the blink-dev thread.
However, as far as I can tell, <applet> is treated as an inline element by
default: in my testing, I wasn't able to observe the rendering differences
without display: block, display: inline-block, or the like.

This seems like something we don't have to discuss -- a jsbin should be an
end-all :)

http://jsbin.com/hocomo/edit?html,output

[dcheng, 2015-09-02 06:31:47]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2015-09-02 06:31:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291723004/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291723004/200001

[commit-bot: I haz the power, 2015-09-02 06:43:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-02 06:43:12]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dcheng, 2015-09-02 06:46:50]

dcheng@chromium.org changed reviewers:
+ haraken@chromium.org

[dcheng, 2015-09-02 06:46:51]

+haraken for Source/bindings changes.

[Nate Chapin, 2015-09-02 17:31:51]

japhet@chromium.org changed reviewers:
+ japhet@chromium.org

[Nate Chapin, 2015-09-02 17:31:52]

Source/bindings/ LGTM

[dcheng, 2015-09-02 17:32:15]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2015-09-02 17:32:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1291723004/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1291723004/200001

[commit-bot: I haz the power, 2015-09-02 18:29:16]

Committed patchset #11 (id:200001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201659