Fix issue #2173 (oembed overlapping top bar) // CSS approach #2319

Merged
merged 1 commit into from Nov 28, 2011

Conversation

Projects
None yet
5 participants
Contributor

rekado commented Nov 5, 2011

The alternative CSS solution described here:
#2206 (comment)

This kills the transparency gradient on the header to keep Flash objects from overlapping.

Contributor

jamiew commented Nov 5, 2011

Interesting, this fixes the overlap issue without needing wmode=transparent?

Contributor

rekado commented Nov 5, 2011

Yes, that's correct. I tested this only in Firefox 7 on GNU/Linux, though.
Personally, I feel that wmode=transparent is the correct way to do it, as modern browsers seem to put embedded objects on the topmost layer by default; controlling the object itself seems ... purer. But then again, adding the wmode line to every o_embed_cache record with a feeble regexp looks hackish, too.

Contributor

manuels commented Nov 6, 2011

Very cool, Recado!

Contributor

jamiew commented Nov 7, 2011

@rekado there is a very significant performance penalty to using wmode=transparent, which is why I was so excited your CSS solution.

I'll try to test this out on other browsers as well. In theory we could do this a wmode=transparent fix on the frontend, inserting it into embeds for browsers as-needed. Safari, for instance, is never affected by this bug (which I see all the time)

Contributor

rekado commented Nov 7, 2011

@jamiew: interesting. I didn't know that this bug doesn't affect Safari. I tried another webkit-based browser (luakit) and the behaviour is the same (i.e. buggy).

Owner

danielgrippi commented Nov 10, 2011

doesn't seem to work in the latest version of chrome (17)...

Contributor

rekado commented Nov 10, 2011

@danielgrippi: makes me a sad monkey. Does this work for Chrome ---> #2206 ? If it does I can make yet another pull request that combines the two. It would only inject the wmode stuff for browsers that need it (actually, I'm not happy with user-agent sniffing); for other browsers the CSS fix will prevent the overlapping.

@jamiew: I have yet to play with wmode=opaque. This might impose less of a performance penalty than transparent.

I'd prefer fixing the oembed records at the time of creation (as in the other pull request). Specifying wmode is really the correct way of doing this. Everything else just relies on browser quirks.

Contributor

Pistos commented Nov 10, 2011

Is it an option to just simplify things by making the header not have a fancy gradient? Or if the gradient is demanded, to use a plain repeated background image?

Contributor

manuels commented Nov 21, 2011

so is this actually working in all browsers?
I think we are spending way to much time for this bug fix.
Either we pull this solution or the non-css solution #2173.
But we should pull one. The implementation details are not crucial in this case.

Sorry, don't mean to be rude.

EDIT: oh, yeah: or this one: #2206 :)

Contributor

rekado commented Nov 22, 2011

@manuels Apparently, this one is not working in Chrome (see @danielgrippi's comment above), but it does work with both webkit (tested luakit) and gecko (tested firefox). The wmode stuff doesn't fix the soundcloud embeds, but the youtube fix works in all browsers.
Pulling this CSS fix would not have any adverse effects, while the wmode fix will lead to poorer performance when flash objects are embedded. I'd love to see this CSS fix pulled and later adjusted to fix issues with Chrome.

Contributor

manuels commented Nov 22, 2011

I agree on that: I think we should pull this, since this is just CSS code and won't break anything.

Any objections?

Owner

danielgrippi commented Nov 28, 2011

alright, let's do it.

danielgrippi merged commit a672dfc into diaspora:master Nov 28, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment