Page MenuHomePhabricator

Floating menu (attached to MWSaveDialog's editSummaryInput by a script/gadget) can be misplaced under certain circumstances
Open, Needs TriagePublic

Description

See screenshot:

I am aware that the circumstances under which I can reproduce this are really hacky, but still I don't see that my hacks are responsible for the misplacing.
What I'm doing is this:

I create a new class for widgets inheriting from TextInputWidget with LookupElement as mixin, and then use a hack to replace the summary input widget in VE's save dialog with an instance of this new widget (https://de.wikipedia.org/wiki/Benutzer:Schnark/js/veSummary.js).

When the summary has some pre-filled value, the menu is misplaced the way shown in the screenshot when the dialog opens. Closing the dropdown (by bluring the input field) and opening it again fixes the position. It is also correct when the input is initially empty and the dropdown only shows once you enter something. Typing while the dropdown is misplaced does not fix the position.

This only started when I decided to pass on the $overlay parameter from the window to the input widget, before this the menu was always positioned correctly.

Event Timeline

Schnark created this task.Jan 23 2017, 9:07 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 23 2017, 9:07 AM
matmarex added a subscriber: matmarex.

When a dialog opens, there is a little animation that shows it fading into view and expanding. Looks like the dropdown is aligned to the initial step of that animation:

Initial frame
End result
Overlaid

I think ve.ui.MWSaveDialog is calling setValue() on the input at unexpected time during dialog opening. Looks like it happens from initialize(), it should probably be done in getSetupProcess()? I haven't looked into it in detail.

Restricted Application added a project: VisualEditor. · View Herald TranscriptJan 23 2017, 6:01 PM

Side note: your script only works on Firefox (the toSource() method is a Firefox extension). But with this one weird trick, it can be made to work everywhere (and I can confirm that the issue you're describing happens on Chromium too):

	HackedMWSaveDialog.prototype.initialize = function () {
		// Define this.editSummaryInput such that the parent method won't be able to override it
		// (but it will still be able to define event handlers and such)
		Object.defineProperty( this, 'editSummaryInput', {
			writable: false,
			value: new EditSummaryWidget( {
				$overlay: this.$overlay,
				// Code below copied from ve.ui.MWSaveDialog.prototype.initialize
				multiline: true,
				placeholder: ve.msg( 'visualeditor-editsummary' ),
				classes: [ 've-ui-mwSaveDialog-summary' ],
				inputFilter: function ( value ) {
					// Prevent the user from inputting newlines (this kicks in on paste, etc.)
					return value.replace( /\r?\n/g, ' ' );
				}
			} )
		} );
		// Parent method
		HackedMWSaveDialog.super.prototype.initialize.call( this );
	}

Further side note: there really should be a supported API to mess with the edit summary textbox. There's at least one other script out there that does it: https://pl.wikipedia.org/wiki/MediaWiki:Gadget-edit-summaries.js

Side note: your script only works on Firefox (the toSource() method is a Firefox extension). But with this one weird trick, it can be made to work everywhere (and I can confirm that the issue you're describing happens on Chromium too):

Thanks for noting. I decided to keep close to my original approach (doesn't duplicate all those other parameters) and just used toString instead of toSource, and adding the parenthesis around it.

Just for the record: I removed the $overlay parameter from my script for now, so the dropdown menu is placed correctly, but limited to the height of the dialog.

@matmarex @Schnark Are there any further action items here?

The VE bug seems like it's still valid. I don't think there's a OOUI bug here. Let's make this about VE.

matmarex renamed this task from Floating menu can be misplaced under certain circumstances to Floating menu (attached to MWSaveDialog's editSummaryInput by a script/gadget) can be misplaced under certain circumstances.May 30 2018, 11:14 PM
matmarex removed a project: OOUI.

JFTR: In my user script I use a different approach now (for different reasons), so I'm no longer affected by this bug. So unless someone else runs into this somewhere else, it probably can be ignored.