Page MenuHomePhabricator

OOjs UI: ClippableElement shouldn't assume the body height is 100%
Closed, ResolvedPublic

Description

user report:
<<In Monobook, when I first open the dialogue and type a character into the "add template" text field, the field will get grey stripes for a second, and then the suggestions drop down menu will appear for a split second but then disappear again (too quick to use). Pressing backspace and trying again doesn't do anything, clicking away and clicking back and trying again doesn't do anything. Closing the dialogue and reopening it will cause the behaviour to repeat, however, but of course to no use. Behaviour is normal with Vector skin. FF 24 Win7. --Atethnekos (Discussion, Contributions) 07:40, 4 October 2013 (UTC)>>
Same for me.


Version: unspecified
Severity: major

Details

Reference
bz54965

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 2:11 AM
bzimport added a project: OOUI.
bzimport set Reference to bz54965.

Just reporting this is still happening.

Also, the same happens when you try to add a category.

Seems to be to do with OO.ui.ClippableElement.prototype.clip using this.$clippableContainer (the <body> of the dialog frame), which if you inspect in developer tools appears to cover no area in monobook but covers the full dialog frame in vector. .innerHeight() returns 0 on monobook so desiredHeight is calculated as a negative number (Which jQuery inserts as 0px).

I think this is because Vector sets height: 100% on body elements, but Monobook doesn't.

Change 117241 had a related patch set uploaded by Alex Monk:
Set body height to 100% in monobook

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

Alex's diagnosis looks correct. The <html> and <body> elements on the inner frame do actually have a height of 0px, because all of their children are (in this particular example, this might not always be the case) absolutely positioned.

Setting their height to 100% in CSS makes <html> span the entire height of its docment (the iframe one in this case) and <body> the entire height of the <html>, solving the issue.

Normally the styles set in the outer document do not apply to documents in <iframe>s, but the OO.ui.Frame.static.transplantStyles method copies them when the frame is created.

Since OOjs UI definitely shouldn't depend on html, body { height: 100% } being present in page's CSS, I see two solutions to this (someone more knowledgeable about the structure of OOjs should decide which one is saner, or some up with a better one):

  • Make OO.ui.Frame.static.transplantStyles set <html>'s and <body>'s height to 100%. This would be trivial to do but feel hacky to me.
  • Use this.$clippableWindow instead of this.$clippableContainer in OO.ui.ClippableElement.prototype.clip when measuring heights, or just pass the window as the container to the constructor in these particular cases. This may or may not be a good idea.

Change 117241 abandoned by Alex Monk:
Set body height to 100% in monobook

Reason:
That was basically what I was expecting, thanks Bartosz

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

Ed has independently applied my second suggestion from March in 8bd708b9d5260defef664346523a6d78c14c4837 two months ago, as a fix for bug 68370 (which is a duplicate of this one), so this looks properly fixed to me.

  • This bug has been marked as a duplicate of bug 68370 ***
  • Bug 68370 has been marked as a duplicate of this bug. ***