Change OOUI EditPage inputs to keep the old 'id' attributes on the <input> elements
Closed, ResolvedPublic1 Story Points

Description

We should change the proposed OOUI EditPage inputs to keep the old 'id' attributes on the <input> elements, like 'wpSummary' or 'wpSubmit'. Right now, it puts them on the wrapper <div>s. This would avoid breaking countless gadgets.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 20 2017, 10:03 AM

Change 354651 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] InputWidget: Introduce #setInputId and 'inputId' config option

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

I imagine that this will break lots of things that are blindly assuming wpSubmit etc. can still be written to as before, though…

For most basic bits, it will mostly keep working – we still use a normal HTML <input> and it can still be interacted with using the usual DOM methods. Things might start behaving funny if we infuse the widget, but the things editors care about (like submitting the form with the edit summary) will work just fine.

Things might start behaving funny if we infuse the widget,

When. ;-(

Change 354653 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] EditPage: Restore the old 'id' attributes in OOUI mode

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

Jdforrester-WMF set the point value for this task to 1.

Change 354651 merged by jenkins-bot:
[oojs/ui@master] InputWidget: Introduce #setInputId and 'inputId' config option

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

Elitre added a subscriber: Elitre.May 22 2017, 3:47 PM

You are aware that this change will probably break these scripts and similar? These are probably much fewer than the number of scripts that will break otherwise, but I just wanted to mention it.

Change 354653 merged by jenkins-bot:
[mediawiki/core@master] EditPage: Restore the old 'id' attributes in OOUI mode

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

You are aware that this change will probably break these scripts and similar? These are probably much fewer than the number of scripts that will break otherwise, but I just wanted to mention it.

Yeah, I guess. I'm afraid it's not possible to work around both cases. And those scripts should not actually break entirely – the extra buttons or whatever will still be inserted in the right location, will not interfere with the existing OOjs UI buttons, and should actually still work (although their styling will be off).

matmarex closed this task as Resolved.Jun 1 2017, 5:55 PM
matmarex removed a project: Patch-For-Review.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJun 1 2017, 5:55 PM
Dvorapa added a subscriber: Dvorapa.Jun 1 2017, 6:53 PM

You are aware that this change will probably break these scripts and similar? These are probably much fewer than the number of scripts that will break otherwise, but I just wanted to mention it.

Yeah, I guess. I'm afraid it's not possible to work around both cases. And those scripts should not actually break entirely – the extra buttons or whatever will still be inserted in the right location, will not interfere with the existing OOjs UI buttons, and should actually still work (although their styling will be off).

My affected script looks actually nicer now, since now there is no longer a gap between the default diff button, and the button I insert after (or rather inside) it. Great, now I have to decide whether to do things the correct way, or the nice way.

My affected script looks actually nicer now, since now there is no longer a gap between the default diff button, and the button I insert after (or rather inside) it. Great, now I have to decide whether to do things the correct way, or the nice way.

Use a ButtonGroupWidget ;) After you get a reference to the existing button, you can treat it like any other widget.

diffButton = OO.ui.infuse( 'wpDiffWidget' );
myButton = new OO.ui.ButtonWidget( { label: 'Woot!' } );
group = new OO.ui.ButtonGroupWidget();
diffButton.$element.after( group.$element );
group.addItems( [ diffButton, myButton ] );

I didn't know it is so easy to get a reference for a button created elsewhere. Thanks!

(To clarify, that only works for buttons/widgets created from PHP code, and only those that have been marked as 'infusable'. You can't use OO.ui.infuse() to get a reference to a widget created by other JS code.)

Majr added a subscriber: Majr.Aug 9 2017, 2:04 PM

So what about everyone on 1.29 who are going to have their scripts broken, and then broken again when they update to 1.30 when the ID is reverted back?

Hmm, that sucks, I forgot that this made it into 1.29. Backporting this would be a little involved, since we'd need the OOjs UI update backported too… Possibly we should just set $wgOOUIEditPage to false in 1.29.

Hmm, that sucks, I forgot that this made it into 1.29. Backporting this would be a little involved, since we'd need the OOjs UI update backported too… Possibly we should just set $wgOOUIEditPage to false in 1.29.

For the sysadmins affected I'd suggest getting them to set it in LocalSettings rather than doing in 1.29.0, reverting in 1.29.1 and un-reverting in 1.30.0 which is confusing, given that most won't be affected.