Page MenuHomePhabricator

[Firefox 73] Infusing a RadioOptionWidget changes first-child alignment
Closed, ResolvedPublicBUG REPORT

Description

Repro steps

  1. Login and visit Special:Preferences with JavaScript enabled on Firefox v73.0 for Linux and a window width >= 1053px in Vector.
    Screenshot_2020-02-20 Preferences - Wikipedia.png (1×2 px, 376 KB)
  2. Open DevTools and infuse the skin group: OO.ui.infuse( $( '#mw-input-wpskin' ) ).
    Screenshot_2020-02-20 Preferences - Wikipedia(1).png (1×2 px, 375 KB)

Notes

Event Timeline

Confirmed; on Firefox v73, even on not too large of a screen.

@Volker_E since this is your patch, do you remember what exactly was fixed? While it's true that it only seems to break in Firefox 73 (no idea why, still checking) we also have the demo that intentionally nullifies that margin.

Also, the infusion in this case switches up the widget. The PHP one is a RadioSelectInputWidget with RadioInputWidget, and after infusion, they transform to RadioSelectWidget with RadioOptionWidget. This is a common thing we see in OOUI for the form elements, but it also means that the design between these two (with the LESS rules) is different as well, which seems to be what makes this only happen after infusion at all. We should unify the design of these two, shouldn't we? At least in the level of the margins, I'd imagine?

Change 566881 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/Vector@master] [Special:Preferences] [PHP] Add skin version user preference and configs

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

Firefox renders it like this because the "Skin" header element (<legend class="oo-ui-fieldsetLayout-header">) has float: left;, so the browser tries to flow the following content around it. Normally the content should be underneath the header so this wouldn't be necessary, but the margin-top: -4px rule which you identified forces it.

For the record, I believe Firefox's rendering is correct here, and Chrome's is incorrect. I also checked this on the Microsoft Edge browsers, and on the old Presto-based Opera, and they both render this the same as Firefox.

Change 574878 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] Avoid wrapping problems with negative margins

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

Turns out the negative margin was just used to cancel out padding… We can do the same more simply by just setting the padding to zero. The result seems to be the same.

Looks good to me. Thank you for figuring it out, @matmarex !

Change 574878 merged by jenkins-bot:
[oojs/ui@master] Avoid wrapping problems with negative margins

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

Change 575031 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/core@master] [CSS] [Special:Preferences] Remove redundant OOUI override

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

Change 566881 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [Special:Preferences] [PHP] Add skin version user preference and configs

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

Volker_E triaged this task as Medium priority.Feb 27 2020, 2:01 AM
Volker_E moved this task from Backlog to OOUI-0.37.0 on the OOUI board.
Volker_E edited projects, added OOUI (OOUI-0.37.0); removed OOUI.

Change 575115 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Update OOUI to v0.37.0

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

Change 575115 merged by jenkins-bot:
[mediawiki/core@master] Update OOUI to v0.37.0

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

Change 575031 merged by jenkins-bot:
[mediawiki/core@master] [CSS] [Special:Preferences] Remove redundant OOUI override

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

matmarex removed a project: Patch-For-Review.

Verified that it looks fine now, when manually infusing on Special:Preferences, and when using Special:GlobalPreferences (T225206).