Page MenuHomePhabricator

Use DOM append instead of z-index to position lightbox
Open, LowPublic


Krinkle says z-index isn't as reliable and that position: absolute should be enough if we're the last thing in the <body> element.

Version: unspecified
Severity: normal
See Also:



Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 2:24 AM
bzimport added a project: MediaViewer.
bzimport set Reference to bz56403.
bzimport added a subscriber: Unknown Object (MLST).
Tgr added a comment.Oct 31 2013, 10:49 PM

I don't think so - anything with a z-index would still be above the lightbox.

Also, AFAIK z-index is reliable enough as long as you are using it on direct children of the <body> - the IE6 bug is related to z-indexed elements having non-static parents.

The z-index on .mlb-fullscreen-div is pointless, however - its parent also has a z-index so that one will determine how it is placed.

I guess the idea here is that we should *all* be using the append-to-DOM method as opposed to the z-index method, because the z-index method is DUMB.

What else is using z-index right now, that would prevent us from moving to append? Isn't it risky to bet on the fact that all other extensions will play nice and not use z-index?

The backdrop, wrapper, and maybe the file usage dialog(s) are all using various values for z-index. Apparently the buttons are, too.

It shouldn'

...t be too hard to fix this, though, it's just a matter of making sure everything works well.

Honestly we could probably display:none the rest of the DOM while the lightbox is open.

...oh, we do already. We don't need z-index for that, then.

Tgr added a comment.Feb 20 2014, 7:43 PM

We are appending to the end of the body (maybe that was different when this bug was opened), but I see no good reason not to have a z-index as well. Any administrator on any wiki can put a z-index rule into MediaWiki:Common.css and cause us problems. Is there ever a valid reason to position something above the lightbox? (Notifications, maybe?)

Tgr added a comment.Feb 20 2014, 7:45 PM

...although display:none works as well.

Yep, display: none is being applied already (could be taken a little bit further, though, by hiding all children except the mmv elements, currently it uses a whitelist). I think we can safely get rid of z-index. Any objections?

Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:12 PM
Restricted Application added subscribers: Matanya, Aklapper. · View Herald TranscriptSep 4 2015, 6:12 PM

Mass-removing the Multimedia tag from MediaViewer tasks, as this is now being worked on by the Reading department, not Editing's Multimedia team.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 27 2016, 4:22 PM
Krinkle removed a subscriber: Krinkle.
Tgr added a comment.Feb 18 2017, 7:52 PM

See T158468: CentralNotice buttons appear over media viewer panel on mobile for an example of why you should not rely on the assumption that no one else uses z-index.

Tgr removed a subscriber: Tgr.Jul 9 2019, 6:05 PM