WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
92320
HTMLAppletElement should inherit from HTMLPlugInImageElement
https://bugs.webkit.org/show_bug.cgi?id=92320
Summary
HTMLAppletElement should inherit from HTMLPlugInImageElement
Anders Carlsson
Reported
2012-07-25 17:00:47 PDT
HTMLAppletElement should inherit from HTMLPlugInImageElement
Attachments
Patch
(5.54 KB, patch)
2012-07-25 17:05 PDT
,
Anders Carlsson
no flags
Details
Formatted Diff
Diff
Patch
(25.34 KB, patch)
2012-07-26 14:30 PDT
,
Anders Carlsson
eric
: review+
webkit-ews
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Anders Carlsson
Comment 1
2012-07-25 17:05:21 PDT
Created
attachment 154495
[details]
Patch
Early Warning System Bot
Comment 2
2012-07-25 17:58:52 PDT
Comment on
attachment 154495
[details]
Patch
Attachment 154495
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/13339446
WebKit Review Bot
Comment 3
2012-07-25 18:32:37 PDT
Comment on
attachment 154495
[details]
Patch
Attachment 154495
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13341441
Early Warning System Bot
Comment 4
2012-07-25 19:10:44 PDT
Comment on
attachment 154495
[details]
Patch
Attachment 154495
[details]
did not pass qt-wk2-ews (qt): Output:
http://queues.webkit.org/results/13345437
Gyuyoung Kim
Comment 5
2012-07-25 19:57:56 PDT
Comment on
attachment 154495
[details]
Patch
Attachment 154495
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/13349460
Gustavo Noronha (kov)
Comment 6
2012-07-25 21:19:09 PDT
Comment on
attachment 154495
[details]
Patch
Attachment 154495
[details]
did not pass gtk-ews (gtk): Output:
http://queues.webkit.org/results/13341491
Anders Carlsson
Comment 7
2012-07-26 14:30:41 PDT
Created
attachment 154753
[details]
Patch
Eric Seidel (no email)
Comment 8
2012-07-26 14:40:30 PDT
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?
Anders Carlsson
Comment 9
2012-07-26 14:41:39 PDT
(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.
Eric Seidel (no email)
Comment 10
2012-07-26 14:43:47 PDT
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?
Eric Seidel (no email)
Comment 11
2012-07-26 14:47:07 PDT
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.
Anders Carlsson
Comment 12
2012-07-26 14:50:14 PDT
(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.
Early Warning System Bot
Comment 13
2012-07-26 15:22:22 PDT
Comment on
attachment 154753
[details]
Patch
Attachment 154753
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/13374026
Anders Carlsson
Comment 14
2012-07-26 16:09:08 PDT
Committed
r123811
: <
http://trac.webkit.org/changeset/123811
>
Csaba Osztrogonác
Comment 15
2012-07-26 23:10:37 PDT
(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."
Anders Carlsson
Comment 16
2012-07-27 09:40:07 PDT
(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" ;)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug