Page MenuHomePhabricator

Clarify usage of HTMLForm::setWrapperAttributes and make it work for OOUI forms
Open, Needs TriagePublic

Description

The documentation states:

* For internal use only. Use is discouraged, and should only be used where
* support for gadgets/user scripts is warranted.
* @internal

It's not entirely clear to me what this means. Also, and most importantly, this option doesn't work for OOUI forms, because it's ignored in the OOUIHTMLForm subclass.

Event Timeline

Can you say what you want to use it for? This documentation comment sounds like deprecation by another name…

Can you say what you want to use it for?

I wanted to add a class to the fieldset so I could target it in CSS. More context: I needed the custom styling to add the "Filter events" button that you can see here. I ended up not needing this method and targeting #form-id fieldset instead (it's not very nice, but you also can't set attributes on the form AFAICS).

This documentation comment sounds like deprecation by another name…

Indeed... According to git blame, it was introduced in r499675, already with that comment. CC'ing @Jdlrobson who authored the commit.

As the commit message suggests "Restore #mw-history-search id on history action fieldset" we had to restore the ID for gadgets/user scripts [1]. The method was marked as @internal to avoid extensions/skins marking up fieldset with additional classes and IDs. It can be used for existing PHP classes in core that want to smoothness the migration to OOUI by not breaking gadgets. This method should not be considered a public API and should not be called unless there's good reason.

On the long term, we should be moving away from ID selectors on every element, so should be looking to replace the #mw-history-search selector with a fieldset selector.

Feel free to modify the wording as needed.

[1] https://global-search.toolforge.org/?q=mw-history-search&regex=1&namespaces=&title=