r66505 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r66504‎ | r66505 | r66506 >
Date:00:26, 16 May 2010
Author:hartman
Status:resolved (Comments)
Tags:
Comment:
Change the width of the div surrouding mediaplayers into min-width

The native a/v players of FireFox and Chrome, have a minimum width that is
rather large (220px). In many cases Wikipedia thumbs are requested at a size
that is smaller than that. With a set width, the elements than overflow out
of fixed width divs. Using min-width instead allows the surrounding div
to resize to fit its contents.

Fixes bug 22538
Modified paths:
  • /trunk/extensions/OggHandler/OggHandler_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/OggHandler/OggHandler_body.php
——@@ -604,7 +604,7 @@
605605 $s = Xml::tags( 'div',
606606 array(
607607 'id' => $id,
608 - 'style' => "width: {$width}px;" ),
 608+ 'style' => "min-width: {$width}px;" ),
609609 ( $thumb ? Xml::tags( 'div', array(), $thumb ) : '' ) .
610610 Xml::tags( 'div', array(),
611611 Xml::tags( 'button',

Follow-up revisions

RevisionCommit summaryAuthorDate
r66567Follow up r66505...hartman15:18, 17 May 2010

Comments

#Comment by Simetrical (talk | contribs)   02:09, 16 May 2010

Did you test this in IE6? It doesn't support min-width, so I'd expect this risks the div filling the whole width of the page.

#Comment by TheDJ (talk | contribs)   13:26, 16 May 2010

if it doesn't support it, it ignores it. Doesn't it size to fit the parent div then ? In most cases the parent div has a a fixed div setting of it's own, and when it doesn't (icon mode for instance) it is in a left float or right floating div, which autosizes it as well. In none float mode it should actually span the entire width.

You could probably even remove the width setting entirely, but this seemed safer to me for now.

#Comment by Simetrical (talk | contribs)   19:48, 16 May 2010

Hmm, you're right. It probably makes more sense to just remove it. Seeing min-width with no IE6 fallback immediately sets off alarm bells for me. If you remove it, it will be clear that the behavior is the same across all browsers. It will also make sure we notice quickly if it was actually present for some good reason, since it will fail in all browsers instead of just IE6.

Status & tagging log