HTMLAppletElement should inherit from HTMLPlugInImageElement
Created attachment 154495 [details] Patch
Comment on attachment 154495 [details] Patch Attachment 154495 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13339446
Comment on attachment 154495 [details] Patch Attachment 154495 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13341441
Comment on attachment 154495 [details] Patch Attachment 154495 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13345437
Comment on attachment 154495 [details] Patch Attachment 154495 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13349460
Comment on attachment 154495 [details] Patch Attachment 154495 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13341491
Created attachment 154753 [details] Patch
So <applet> is only for java-plugin content, right? And <embed> is only for other plugins? And <object> is for both plugins and images? Is that correct?
(In reply to comment #8) > So <applet> is only for java-plugin content, right? And <embed> is only for other plugins? And <object> is for both plugins and images? Is that correct? This is partly true, except that Java applets can be instantiated with <embed> and <object> as well.
So it seems the only trick in validating this patch is that we didn't give <applet> too many powers its not supposed to have, right?
Comment on attachment 154753 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154753&action=review This seems reasonable to me. It's very difficult to verify that this is correct from just seeing the code. I hope we have testing... :) > Source/WebCore/html/HTMLAppletElement.cpp:42 > + : HTMLPlugInImageElement(tagName, document, createdByParser, ShouldNotPreferPlugInsForImages) ShouldNotPreferPlugins? > Source/WebCore/html/HTMLAppletElement.cpp:153 > + renderer->setWidget(frame->loader()->subframeLoader()->createJavaAppletWidget(roundedIntSize(LayoutSize(contentWidth, contentHeight)), this, paramNames, paramValues)); Do we worry that we're now going to be exposing a different set of params to the java plugin? It's not clear to me that we are, but is one possible concern here.
(In reply to comment #11) > (From update of attachment 154753 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154753&action=review > > This seems reasonable to me. It's very difficult to verify that this is correct from just seeing the code. I hope we have testing... :) > > > Source/WebCore/html/HTMLAppletElement.cpp:42 > > + : HTMLPlugInImageElement(tagName, document, createdByParser, ShouldNotPreferPlugInsForImages) > > ShouldNotPreferPlugins? It doesn't really matter - it's impossible for an <applet> tag to end up creating an image. > > > Source/WebCore/html/HTMLAppletElement.cpp:153 > > + renderer->setWidget(frame->loader()->subframeLoader()->createJavaAppletWidget(roundedIntSize(LayoutSize(contentWidth, contentHeight)), this, paramNames, paramValues)); > > Do we worry that we're now going to be exposing a different set of params to the java plugin? It's not clear to me that we are, but is one possible concern here. Nah, the only thing that could break was if we had multiple parameters with the same names and different values but doing that is undefined behavior anyway.
Comment on attachment 154753 [details] Patch Attachment 154753 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13374026
Committed r123811: <http://trac.webkit.org/changeset/123811>
(In reply to comment #14) > Committed r123811: <http://trac.webkit.org/changeset/123811> And Qt buildfix landed in http://trac.webkit.org/changeset/123838 Why do you break Qt build regularly and intentionally? Qt EWS was red, EWS commented the bug, but you landed your wrong patch. Please don't do that next time. https://www.webkit.org/coding/contributing.html "Your change must at least compile on all platforms."
(In reply to comment #15) > (In reply to comment #14) > > Committed r123811: <http://trac.webkit.org/changeset/123811> > > And Qt buildfix landed in http://trac.webkit.org/changeset/123838 > > Why do you break Qt build regularly and intentionally? Qt EWS was red, > EWS commented the bug, but you landed your wrong patch. Please don't do > that next time. > I did fix the error the bot reported, but there were more errors that weren't reported by the EWS bot. The code is correct - this is a bug in the compiler which is why none of the other builds broke. Maybe if you switched to a better compiler I'd stop "intentionally breaking the build" ;)