367247 - Plugin placeholders should be inline-block

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Attachment: 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.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, they were intended to be in bug 309521; I'm surprised nobody filed a followup bug then.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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).

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This fixes inline-blocks so they support overflow, and adds some helpers on nsStyleDisplay that I've had in my tree for ages.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 252844 [details] [diff] [review]
patch

>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?

sr=bzbarsky.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 252844 [details] [diff] [review]
patch

r=biesi

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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...

Comment 9

[Martijn Wargers (dead)]

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.

Comment 10

[Jeff Walden [:Waldo]]

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/reftests&command=DIFF_FRAMESET&root=/cvsroot&file=reftest.list&rev1=1.32&rev2=1.33