Bug 15081

Summary:

Plug-ins display despite having display: none

Product:

WebKit

Reporter:

mitz

Component:

Plug-ins

Assignee:

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:

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-	Details Formatted Diff Diff
New patch for bug 15081 (2.06 KB, patch) 2008-11-06 09:27 PST, Lyon Chen	mrowe: review-	Details Formatted Diff Diff
Patch, testcases, changelog (10.53 KB, patch) 2009-02-24 22:10 PST, Simon Fraser (smfr)	andersca: review+	Details Formatted Diff Diff
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.