368972 - Default width/height for plugin placeholders should be 240px/200px

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

This is a follow-up from bug 367247, the testcase is from that bug.
The last example of the testcase has no width/height set.
In that case the width/height of the plugin placeholder should be 240px/200px.
I guess this could be fixed in missingPlugin.css, but biesi mentioned that the embed element should get this width/height from:
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsObjectFrame.cpp#596

Comment 1

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

Yeah, after thinking more about that I was wrong. I broke that when I rewrote pluginfinder. This should be sized explicitly now.

Comment 2

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

> This should be sized explicitly now.

meaning: CSS should be added somewhere to do that sizing.

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase1

Comment 4

[Martijn Wargers (dead)]

Attachment: testcase2

Comment 5

[Martijn Wargers (dead)]

Attachment: patch?

I think this fixes it for most cases, except it doesn't fix the last case of testcase1, where width="xxx".
Basically, I need a selector(s) that fo all plugin elements that don't have a (valid) width/height set. It seems to me that isn't possible with css.

Comment 6

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

Comment on attachment 254830 [details] [diff] [review]
patch?

hm, if this works r=biesi

Comment 7

[Martijn Wargers (dead)]

Comment on attachment 254830 [details] [diff] [review]
patch?

Well, yes, it works, except for the width="xxx" case.
But I guess that's probably not a big deal.

Comment 8

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

(In reply to comment #5)
> Basically, I need a selector(s) that fo all plugin elements that don't have a
> (valid) width/height set. It seems to me that isn't possible with css.

Or you need a way of making your style rules apply at a level of the cascade lower than the width/height attributes.

Comment 9

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

Comment on attachment 254830 [details] [diff] [review]
patch?

sr=dbaron -- good enough for now, but please file a bug on doing it right (so it applies to any invalid height/width attribute by being at a lower level of the cascade)

Comment 10

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

Comment on attachment 254830 [details] [diff] [review]
patch?

a19=dbaron

Comment 11

[Martijn Wargers (dead)]

Checking in missingPlugin.css;
/cvsroot/mozilla/toolkit/themes/winstripe/mozapps/plugins/missingPlugin.css,v  <
--  missingPlugin.css
new revision: 1.4; previous revision: 1.3

Checked into trunk.

I filed bug 390700 as a follow-up for a better solution as was suggested by dbaron in comment 9.

Comment 12

[Dave Townsend [:mossop]]

Attachment: pinstripe patch and mochitest

Apparently this fix wasn't applied to pinstripe. This fixes that and adds a mochitest for it using the example testcases.

Comment 13

[Dave Townsend [:mossop]]

I'll fix the alignment oddity in Makefile.in on checkin.

Comment 14

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

Comment on attachment 337869 [details] [diff] [review]
pinstripe patch and mochitest

r+sr=dbaron

Comment 15

[Dave Townsend [:mossop]]

Landed http://hg.mozilla.org/mozilla-central/rev/0d7bbeb6caa3