Page MenuHomePhabricator

jquery dialog with auto size takes 50% screen width in RTL language wikis
Open, Stalled, NormalPublic

Description

There is a new issue with version 1.25wmf14 (6d956f6), in which jquery dialogs with auto size in RTL languages are "broken", e.g instead of auto width it takes 50% of the screen very ugly and annoying.

How to test:

  1. go to Hebrew/Arabic wikipedia
  2. get to wikitext edit
  3. press on the "{{ }}" icon on the toolbar (this opens a dialog with auto width) - ugly window should appear

another way to get it (just for debug)

  1. Get to https://en.wikipedia.org/w/index.php?title=jqueryUI&action=edit&uselang=ar
  2. open cite on the toolbar
  3. select some template
  4. change its css width from some value to auto (this isn't assumed to be regular/valid user flow just to convince this isn't a issue with specific script in he/ar wiki)

Possible solution:
tell the resource loader @noflip on .ui-dialog { left: 0 } rule in lib/jquery.ui/themes/smoothness/jquery.ui.dialog.css

I'm not sure:

  1. if it is OK to do it for imported external library or there is a better option so we shouldn't have to copy it in next jquery ui version
  2. Why this happens only in the last mw version (didn't see in the patch logs something relevant to jquery ui) and why it doesn't happen with the regular dialgos with Wikieditor (I guess this use explicit width, but not sure)

Event Timeline

eranroz created this task.Jan 15 2015, 11:10 PM
eranroz updated the task description. (Show Details)
eranroz raised the priority of this task from to Needs Triage.
eranroz added a subscriber: eranroz.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 15 2015, 11:10 PM

Added local fix for hewiki common.css:
.ui-dialog { right: auto; }

(e.g to get rid of the wrong right style rule)

Aklapper renamed this task from jquery dialog with auto size are broken in RTL languages to jquery dialog with auto size takes 50% screen width in RTL language wikis.Jan 16 2015, 2:01 PM
Aklapper triaged this task as Normal priority.
eranroz raised the priority of this task from Normal to High.

The Vector skin has its own jQuery UI theme, which was updated somewhat recently.

So I guess that it should be something like

.ltr .ui-dialog {
    left: 0;
}
.rtl .ui-dialog {
    right: 0;
}

But the question is where to put it.

  • Patching upstream jquery.ui makes sense, but .ltr and .rtl are MediaWiki classes and I'm not sure that they will accept a patch with [dir=ltr] and [dir=rtl] selectors.
  • Patching skins/Vector/skinStyles/jquery.ui/jquery.ui.dialog.css seems like a problematic fork, and doesn't apply to other skins.
  • Adding another CSS file that fixes issues in other skins is ugly.

@Krinkle, any suggestions?

Amire80 added a subscriber: I18n.Jan 18 2015, 3:05 PM

@Krinkle, any suggestions?

Krinkle added a comment.EditedFeb 16 2015, 5:17 PM

So I guess that it should be something like

.ltr .ui-dialog {
    left: 0;
}
.rtl .ui-dialog {
    right: 0;
}

But the question is where to put it.

  • Patching upstream jquery.ui makes sense, but .ltr and .rtl are MediaWiki classes and I'm not sure that they will accept a patch with [dir=ltr] and [dir=rtl] selectors.
  • Patching skins/Vector/skinStyles/jquery.ui/jquery.ui.dialog.css seems like a problematic fork, and doesn't apply to other skins.
  • Adding another CSS file that fixes issues in other skins is ugly.

    @Krinkle, any suggestions?

@Amire80 You'll have to verify what works best, but I don't think setting right: 0 for RTL will work here. Setting right: 0 for RTL wouldn't do anything because that's what caused this bug in the first place. The stylesheet has left: 0; and CSSJanus flips it to right: 0; in RTL. Then, at run time, jQuery UI adds a top: (something)px rule based on current screen size. Ending up with the dialog stuck to the right edge of the page instead of being centred.

I expect adding @noflip to the left: 0 would solve this bug and is an acceptable solution. While patching upstream libraries should be avoided at all cost, instrumentation like this to prevent it from being interpreted as a CSSJanus instruction is (however unfortunate) common practice. We've added @nofip rules to our copy of jQuery UI in the past already, as well as @embed rules to allow data URI optimisation. Not applying it to this dialog rule was was an oversight.

As for where: Patch the file in MediaWiki core (for fallback of all skins), and the Vector theme specific one. In both cases, be sure to document it in the PATCHES file.

Krinkle changed the task status from Open to Stalled.Apr 28 2015, 5:35 PM

Awaiting feedback from I18n about what the desired course of action is.

Krinkle lowered the priority of this task from High to Normal.Apr 28 2015, 5:36 PM
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.
Restricted Application added a project: I18n. · View Herald TranscriptJun 2 2015, 2:20 PM

Awaiting feedback from I18n about what the desired course of action is.

@Krinkle: How can you await feedback from a tag? Did you mean to CC a team, like Language Engineering, instead?

Amire80 moved this task from Backlog to Other on the RTL board.Jul 16 2015, 3:29 PM
Amire80 moved this task from Other to MediaWiki-core on the RTL board.Aug 30 2015, 7:42 AM
Mjbmr added a subscriber: Mjbmr.Aug 30 2015, 9:22 AM

When you change direction, right will be left and left will be right, so if you have a left value in css, it's absolute and must be a right value instead, and I only seen this behavior in IE, see my trick:

	form.dialog({
		drag: function(event, ui) {
			$(this).parent().css('right', ($(window).innerWidth() - parseInt($(this).parent().css('left')) - $(this).parent().width())+'px');
			$(this).parent().css('left', '');
		},
	})

(Reclassifying since this is not an issue with the ResourceLoader framework itself)

Isarra moved this task from Backlog to Bugs on the MediaWiki-Interface board.Nov 10 2015, 10:07 PM
Krinkle removed a subscriber: Krinkle.May 23 2016, 8:04 PM
Danny_B removed subscribers: Language-Team, I18n.
Guycn2 removed a subscriber: Guycn2.Jul 12 2016, 5:28 PM
Amire80 moved this task from Untriaged to RTL on the I18n board.Feb 28 2018, 12:17 PM
Guycn2 added a subscriber: Guycn2.Jul 11 2018, 4:42 AM