Bug 15081

Summary: Plug-ins display despite having display: none
Product: WebKit Reporter: mitz
Component: Plug-insAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Major CC: andersca, ap, lyon.chen, mihnea, mike.capp, shadow2531, simon.fraser, staikos, tqsub, weihong.zeng
Priority: P2 Keywords: HasReduction, InRadar
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Test case
none
reduction: object with specific size to not display
none
Patch to fix bug 15081.
mjs: review-
New patch for bug 15081
mrowe: review-
Patch, testcases, changelog andersca: review+

mitz
Reported 2007-08-26 07:27:56 PDT
WebKit displays <object> and <embed> plug-ins even if their display is set to "none" (either initially or dynamically). Firefox and Opera do not.
Attachments
Test case (301 bytes, text/html)
2007-08-26 07:28 PDT, mitz 		no flags
Details
reduction: object with specific size to not display (398 bytes, text/html)
2008-07-01 17:16 PDT, Thom 		no flags
Details
Patch to fix bug 15081. (533 bytes, patch)
2008-11-05 07:49 PST, Lyon Chen 		mjs: review-
DetailsFormatted DiffDiff
New patch for bug 15081 (2.06 KB, patch)
2008-11-06 09:27 PST, Lyon Chen 		mrowe: review-
DetailsFormatted DiffDiff
Patch, testcases, changelog (10.53 KB, patch)
2009-02-24 22:10 PST, Simon Fraser (smfr) 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
mitz
Comment 1 2007-08-26 07:28:40 PDT
Created attachment 16124 [details] Test case
mitz
Comment 2 2007-08-26 07:29:33 PDT
See also bug 14339.
Mark Rowe (bdash)
Comment 3 2007-08-26 23:29:04 PDT
<rdar://problem/5439208>
Alexey Proskuryakov
Comment 4 2008-06-18 22:34:20 PDT
*** Bug 19646 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 5 2008-06-18 22:34:23 PDT
*** Bug 10234 has been marked as a duplicate of this bug. ***
Alexey Proskuryakov
Comment 6 2008-06-18 22:35:32 PDT
The duplicates have many additional test cases.
Thom
Comment 7 2008-06-27 12:01:03 PDT
Shouldn't the "Component" be changed from Plug-ins to CSS instead since the plug-ins are working but the CSS style isn't working? Definitely has reductions, so how about adding the HasReduction keyword? This bug also appears with OS as Mac OS X 10.5, and in Versions 525.x (Safari 3.1) and the nightly build. Have also known this bug to be around since well before that, near the beginning of Safari.
Alexey Proskuryakov
Comment 8 2008-06-28 01:39:55 PDT
I think that Plug-ins is a more suitable component for this - not that it matters much.
Thom
Comment 9 2008-07-01 17:16:02 PDT
Created attachment 22036 [details] reduction: object with specific size to not display This test case is similar to one of the test cases in duplicate bug 19646 but reduced more. That one had the style declared inline as well as in a style tag. This reduction does not, so the style is declared only once.
Thom
Comment 10 2008-07-01 17:21:12 PDT
Kind of curious. When I right-click and inspect the element on the test page, it shows in the computed styles that the display property for the object tag has been successfully set to "none". This is also true in the other tests for when the style isn't inline. It would seem the display style is being accepted (not ignored entirely) for the object tag, but for some reason not acted on in a way that affects the display style of the content of the object tag.
Mike Capp
Comment 11 2008-09-08 10:26:54 PDT
This bug also manifests on Windows (testing with 2003 Server), both in Safari 3.1.1 and Chrome 0.2.149.27. Raised http://code.google.com/p/chromium/issues/detail?id=1415 - I'll link it to here.
George Staikos
Comment 12 2008-11-04 08:26:50 PST
Smaller/simpler testcase: <html><body><object style='display: none'></object>This should be at the TOP of the page</body></html>
Lyon Chen
Comment 13 2008-11-05 07:49:49 PST
Created attachment 24909 [details] Patch to fix bug 15081. This patch should fix the display:none style issue for html object tag.
Maciej Stachowiak
Comment 14 2008-11-05 08:11:46 PST
Comment on attachment 24909 [details] Patch to fix bug 15081. Issues with this patch: 1) There shouldn't be a test case in a comment in the code. 2) There *should* be a test case (and expected result) added to LayoutTests. 3) The path contains tabs - it should not. We use all spaces for formatting. 4) I think it would be better style to call HTMLPluginElement::rendererIsNeeded(style) at the end instead of checking style->display() directly. If this was done, then the function could be refactored to do the special case frame check first (if not using fallback or an image) and then call the base class, which would be more elegant. However, the change looks substantively correct to me, given mitz's test case. Please fix at least 1-3, 4 is optional.
Maciej Stachowiak
Comment 15 2008-11-05 08:13:15 PST
Comment on attachment 24909 [details] Patch to fix bug 15081. r=me assuming at least items 1-3 from my list are fixed before committing, and a ChangeLog is added.
Maciej Stachowiak
Comment 16 2008-11-05 08:13:56 PST
Comment on attachment 24909 [details] Patch to fix bug 15081. r- for now since a new patch is coming.
Lyon Chen
Comment 17 2008-11-06 09:27:40 PST
Created attachment 24944 [details] New patch for bug 15081 Changes made following comments by Maciej Stachowiak and George Staikos.
Geoffrey Garen
Comment 18 2008-11-24 22:25:44 PST
Looks good, but: 1. Please "run-webkit-tests css1/classification/display_object.html" in order to generate expected results for your new test, and include the results in your patch. Otherwise, if somebody lands your patch, it will turn the buildbot red. 2. Please add a newline to the end of LayoutTests/css1/classification/display_object.html, to make our diffing tools happy.
Mark Rowe (bdash)
Comment 19 2009-01-30 05:16:08 PST
Comment on attachment 24944 [details] New patch for bug 15081 Marking as r-. A revised patch that addresses Geoff's two points in comment #18 will surely get r+d.
Simon Fraser (smfr)
Comment 20 2009-02-24 22:08:56 PST
*** Bug 23019 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 21 2009-02-24 22:10:50 PST
Created attachment 27959 [details] Patch, testcases, changelog This patch fixes display:none for applet, embed and object.
Anders Carlsson
Comment 22 2009-02-24 22:15:43 PST
Comment on attachment 27959 [details] Patch, testcases, changelog r=me
Simon Fraser (smfr)
Comment 23 2009-02-24 22:18:34 PST
http://trac.webkit.org/changeset/41209
Simon Fraser (smfr)
Comment 24 2011-11-02 16:39:39 PDT
Bug 45049 plans to undo this.
Note You need to log in before you can comment on or make changes to this bug.