Page MenuHomePhabricator

Pluginize fullscreen handling
Closed, ResolvedPublic

Description

Details of cross-browser fullscreen detection/switching should go into a reusable plugin.


Version: unspecified
Severity: enhancement
Whiteboard: gci2013

Details

Reference
bz56477

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 2:30 AM
bzimport set Reference to bz56477.
bzimport added a subscriber: Unknown Object (MLST).
Tgr created this task.Nov 1 2013, 5:50 PM

Thanks, Gergo. I'm all in favor of a cross-browser full-screen plugin, if it can help address some of the issues we are experiencing with inconsistent full-screen behavior. Can you elaborate a bit on your proposed solution?

Tgr added a comment.Nov 5 2013, 10:58 AM

This wouldn't change anything functionally, just make the code cleaner (and avoid the need for code duplication if there are other MediaWiki projects which need to use fullscreen).

Yeah, this would be suuuper simple. And it's a core bug! I'd say file a separate one in MediaWiki/JavaScript and make this depend on it.

Tgr added a comment.Nov 20 2013, 11:35 AM

Unassigning, seems like a good Code-in candidate.

Thinking about this. Would it make more sense to use a pre-existing jQuery + fullscreen plugin rather than port our current (pretty crappy) code? Wheel reinvention is generally not a good thing...especially if our goal is cross-browser compatibility...

Although, speak of the devil, I think I just wrote a half-decent fullscreen plugin myself. Will test, create a patch, and then we can see how things stand. :)

Tgr added a comment.Dec 3 2013, 8:46 AM

Fullscreen handling is very simple (plugins mostly just take care of browser prefixes), I don't think it is worth adding an external dependency. (For reference, here is an existing plugin: https://github.com/kayahr/jquery-fullscreen-plugin/blob/master/jquery.fullscreen.js )

(In reply to comment #7)

Fullscreen handling is very simple

Yeah, that's what I discovered. :)

(as well as learning through trial and error that our current prefix/caps handling had some flaws, apparently...)

Change 99021 had a related patch set uploaded by Theopolisme:
[WIP] Create jquery.fullscreen

https://gerrit.wikimedia.org/r/99021

Change 99021 merged by jenkins-bot:
Create jquery.fullscreen

https://gerrit.wikimedia.org/r/99021

Gilles raised the priority of this task from Normal to Unbreak Now!.Dec 4 2014, 10:24 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to Normal.Dec 4 2014, 11:21 AM