Use of `multiline: true` in subclasses of TextInputWidget broken
Closed, DeclinedPublic
Actions

Description

The same thing that hit VE in T169272: [Regression pre-wmf.8] VE crashes while opening Gallery and some other dialogs also broke CX2 translation view:

load.php?debug=true&lang=fi&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0otr805:3869 Uncaught TypeError: Cannot read property 'toggleClass' of undefined
    at mw.cx.widgets.PageTitleWidget.OO.ui.Widget.setDisabled (Widget.js:84)
    at mw.cx.widgets.PageTitleWidget.OO.ui.InputWidget.setDisabled (InputWidget.js:210)
    at new mw.cx.widgets.PageTitleWidget (mw.cx.ui.PageTitleWidget.js?e79c0:22)
    at mw.cx.ui.SourceColumn.render (mw.cx.ui.SourceColumn.js?6d862:48)
    at mw.cx.ui.SourceColumn.init (mw.cx.ui.SourceColumn.js?6d862:32)
    at new mw.cx.ui.SourceColumn (mw.cx.ui.SourceColumn.js?6d862:24)
    at new MwCxUiColumns (mw.cx.ui.Columns.js?7d227:12)
    at mw.cx.ui.TranslationView.render (mw.cx.ui.TranslationView.js?dbb0e:45)
    at mw.cx.ui.TranslationView.init (mw.cx.ui.TranslationView.js?dbb0e:38)
    at new mw.cx.ui.TranslationView (mw.cx.ui.TranslationView.js?dbb0e:24)

I'm filing this task as I cannot yet see expected follow-ups: Either fix the regression or proactively find the places that are broken and alert the maintainers and guide them with fixing.

Details

	Subject	Repo	Branch	Lines +/-
	Let PageTitleWidget inherit from OO.ui.MultilineTextInputWidget	mediawiki/extensions/ContentTranslation	master	+7 -5

Customize query in gerrit

Related Objects

Mentioned Here: T169272: [Regression pre-wmf.8] VE crashes while opening Gallery and some other dialogs

Event Timeline

Nikerabbit created this task.Jul 4 2017, 9:29 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 4 2017, 9:29 AM

• santhosh subscribed.Jul 4 2017, 9:39 AM

Change 363288 had a related patch set uploaded (by Santhosh; owner: Santhosh):
[mediawiki/extensions/ContentTranslation@master] Let PageTitleWidget inherit from OO.ui.MultilineTextInputWidget

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

gerritbot added a project: Patch-For-Review.Jul 5 2017, 5:00 AM

• santhosh claimed this task.Jul 5 2017, 5:01 AM

• santhosh triaged this task as High priority.

• santhosh added projects: ContentTranslation, Language-2017-July-Sept.

• santhosh moved this task from Backlog to In Review on the Language-2017-July-Sept board.

Change 363288 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@master] Let PageTitleWidget inherit from OO.ui.MultilineTextInputWidget

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

Amire80 moved this task from Needs Triage to CX2 on the ContentTranslation board.Jul 5 2017, 10:07 AM

matmarex renamed this task from Use of multiline: true broken to Use of `multiline: true` in subclasses of TextInputWidget broken.Jul 5 2017, 10:09 PM

matmarex subscribed.Jul 5 2017, 11:29 PM

proactively find the places that are broken and alert the maintainers and guide them with fixing

Okay. I grepped all our code for potential problems:

extensions/Flow/modules/editor/editors/visualeditor/ui/widgets/mw.flow.ve.ui.MentionTargetInputWidget.js:
   42: 	OO.inheritClass( mw.flow.ve.ui.MentionTargetInputWidget, OO.ui.TextInputWidget );

extensions/MobileFrontend/resources/mobile.categories.overlays/CategoryLookupInputWidget.js:
   22: 	OO.inheritClass( CategoryLookupInputWidget, OO.ui.TextInputWidget );

extensions/VisualEditor/lib/ve/src/ui/widgets/ve.ui.WhitespacePreservingTextInputWidget.js:
   35: OO.inheritClass( ve.ui.WhitespacePreservingTextInputWidget, OO.ui.TextInputWidget );

extensions/VisualEditor/modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js:
   41: OO.inheritClass( ve.ui.MWCategoryInputWidget, OO.ui.TextInputWidget );

extensions/WikibaseLexeme/resources/widgets/ItemSelectorWidget.js:
   23: 	OO.inheritClass( ItemSelectorWidget, OO.ui.TextInputWidget );

Dear developers of Flow, MobileFrontend, VisualEditor and Wikibase (hopefully this reaches you, as I added the projects to subscribers): Please ensure that none of these widgets are ever constructed with multiline: true, as they will throw horrible exceptions in that case. If they do need to be multiline, they should inherit from OO.ui.MultilineTextInputWidget instead. Sorry for the mess :(

I think it is not worthwhile for us to add additional code to deal with this deprecated case, so I'm declining this for OOjs UI.

Pginer-WMF subscribed.Jul 6 2017, 6:32 AM

ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-07-11_(1.30.0-wmf.9)).Jul 8 2017, 3:01 AM

Volker_E moved this task from Backlog to Invalid/Declined/Duplicate on the OOUI board.Jul 17 2017, 5:52 PM

Volker_E removed a project: Patch-For-Review.

Volker_E removed a subscriber: gerritbot.

Nikerabbit moved this task from In Review to Done on the Language-2017-July-Sept board.Aug 22 2017, 7:17 AM