Closed Bug 367247 Opened 17 years ago Closed 17 years ago

Plugin placeholders should be inline-block


(Core :: Layout, defect)

Not set





(Reporter: martijn.martijn, Assigned: dbaron)



(Keywords: testcase, Whiteboard: [patch])


(2 files, 1 obsolete file)

Attached file testcase
See testcase.
It seems to me that plugin placeholders should have a inline-block layout, because the embed element is a replaced element.
Also, it seems to me that the embed element should have a default width/height when no width/height is set and the plugin placeholder is rendered.

In Firefox2, the default width/height for plugin placeholders seems to be 240px by 200px. But maybe the same as for iframe should be used, eg 300px by 150px?

Inline-block is implemented in bug 9458.
Yeah, they were intended to be in bug 309521; I'm surprised nobody filed a followup bug then.
Blocks: 309521
Depends on: inline-block
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dbaron
Attachment #252817 - Flags: superreview?(bzbarsky)
Attachment #252817 - Flags: review?(cbiesinger)
Whiteboard: [patch]
OS: Windows XP → All
Hardware: PC → All
Actually, it occurs to me that I probably want vertical-align to still work, which means I want vertical-align: inherit and overflow:hidden (the latter to cause vertical-align:baseline to do bottom-to-baseline alignment rather than baseline-to-baseline alignment).
Attachment #252817 - Flags: superreview?(bzbarsky)
Attachment #252817 - Flags: review?(cbiesinger)
Attached patch patchSplinter Review
This fixes inline-blocks so they support overflow, and adds some helpers on nsStyleDisplay that I've had in my tree for ages.
Attachment #252817 - Attachment is obsolete: true
Attachment #252844 - Flags: superreview?(bzbarsky)
Attachment #252844 - Flags: review?(cbiesinger)
Comment on attachment 252844 [details] [diff] [review]

>diff -r 88ad983e4579 layout/style/nsStyleStruct.h
>+  PRBool IsBlockInside() const {
>+    // Should TABLE_CELL and TABLE_CAPTION go here?  They have
>+    // block frames nested inside of them.

If we add captions here, we'll need to change the frame ctor code that builds scrollframes and then a block for things that are block-inside and have scrollable overflow...

Add a note about that to this comment?

Add tests for the scrolling, if possible?

Attachment #252844 - Flags: superreview?(bzbarsky) → superreview+
Note that what gets checked in for this bug will be addressing one of the review comments in bug 9458: see bug 9458 comment 92.
Comment on attachment 252844 [details] [diff] [review]

Attachment #252844 - Flags: review?(cbiesinger) → review+
Patch checked in (in 2 pieces) 2007-01-27 10:44 -0800 and 2007-01-27 10:46 -0800.

Change mentioned in comment 6 checked in 2007-01-27 10:45 -0800 and partially backed out 2007-01-27 11:35 -0800.

Additional tests per comment 5 checked in 2007-01-31 18:13 -0800.

Note also that the comment I added was more general, and just said to look at callers, which really ought to be obvious when changing a method like that...
Closed: 17 years ago
Resolution: --- → FIXED
I filed bug 368972 on the issue that not the correct width/height is set for the plugin placeholder.

Verified fixed with 2007-01-31 trunk build.
Blocks: 323457
You need to log in before you can comment on or make changes to this bug.