WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 7 months ago

#22935 new defect (bug)

Horizontal scrollbar in RTL media modal

Reported by: ramiy Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: needs-patch
Focuses: rtl Cc:

Description

The new media tab needs a few RTL adjustments on the UI, check out the screenshot.

Attachments (9)

media_RTL_search.png (55.1 KB) - added by ramiy 2 years ago.
22935.media-rtl.png (27.6 KB) - added by SergeyBiryukov 2 years ago.
22935.publish-opera.png (7.9 KB) - added by SergeyBiryukov 2 years ago.
22935.patch (416 bytes) - added by SergeyBiryukov 2 years ago.
22935.media-rtl.2.png (55.1 KB) - added by SergeyBiryukov 2 years ago.
22935.media-rtl.3.png (52.4 KB) - added by SergeyBiryukov 2 years ago.
22935.1.diff (821 bytes) - added by lessbloat 2 years ago.
22935.2.patch (530 bytes) - added by ocean90 21 months ago.
22935.attachment-browser.png (281.3 KB) - added by ocean90 7 months ago.

Download all attachments as: .zip

Change History (35)

@ramiy2 years ago

comment:1 @ramiy2 years ago

  1. On the search "input" box, move the X to left.
  1. Remove the horizontal and vertical scroll bars.

comment:2 @nacin2 years ago

  • Milestone changed from Awaiting Review to 3.5.1

comment:3 follow-ups: @SergeyBiryukov2 years ago

  • Keywords reporter-feedback added

Replying to ramiy:

  1. On the search "input" box, move the X to left.

Which browser is that? Tested in Firefox 17, Chrome 23, IE 8, Opera 12, didn't see the X at all: 22935.media-rtl.png.

  1. Remove the horizontal and vertical scroll bars.

Could not reproduce the scroll bars either.

However, I've noticed an overlapping icon in the Publish box in Opera: 22935.publish-opera.png.

comment:4 in reply to: ↑ 3 @JDTrower2 years ago

I can confirm that the X appears in Chrome 23 and Safari 5.1.7 for Windows. However, it only appears after you have started typing a search term in the search box. I could not reproduce in Firefox 17, Firefox 10 ESR, IE 9, or Opera 12.

However, I was unable to reproduce the scroll bars issue, as reported by the original reporter.

I did notice the overlapping icon in Opera as well.

comment:5 in reply to: ↑ 3 ; follow-up: @ramiy2 years ago

Replying to SergeyBiryukov:

To reproduce the X on the search "input" box, start typing your search term, using Chrome 23.

comment:6 @SergeyBiryukov2 years ago

  • Keywords reporter-feedback removed

I can now reproduce the scroll bars in Chrome by reducing the height of browser window a bit.

@SergeyBiryukov2 years ago

comment:7 in reply to: ↑ 5 @SergeyBiryukov2 years ago

Replying to ramiy:

To reproduce the X on the search "input" box, start typing your search term, using Chrome 23.

Appears to be a Chromium bug, fixed two weeks ago: ​https://codereview.chromium.org/11421088/.

Still, 22935.patch fixes that as well.

comment:8 follow-ups: @helen2 years ago

The X on the search input box happens everywhere we have type="search" for inputs in the admin. That's probably better addressed all at once, in a separate ticket, and not for 3.5.1.

For the scrollbars, the vertical one should probably stay. I can't reproduce the horizontal one in OSX, but if the source of that can be found, then we can look at that. Would be good to know if that's RTL-specific or not.

comment:9 in reply to: ↑ 8 @JDTrower2 years ago

Replying to helen:

The X on the search input box happens everywhere we have type="search" for inputs in the admin. That's probably better addressed all at once, in a separate ticket, and not for 3.5.1.

I have checked a default install of 3.5 and of trunk (both regular LTR install and a RTL install with no patches applied), and there is no X in the search box in the add media when using Firefox 17, Firefox 10 ESR, IE 9, or Opera 12. There is a X in Chrome 23 and Safari 5. All testing was done on a fully updated Windows 7 Ultimate computer.

I'll have to find the other search boxes and test them to see if the same thing occurs in all search boxes, or just the search box in add media.

comment:10 @JDTrower2 years ago

I just did further testing of the search boxes found on the All Posts, Categories, Tags, Media Library, Pages, Comments, Install Themes, Plugins, Add New Plugins, and Users pages and had the same results as I did when checking the Add Media that I posted in comment 9 above.

comment:11 @helen2 years ago

Right, it's a Webkit thing, and the direction issue with the X has been happening all along ever since the introduction of type="search" fields in 3.4, so it's not for 3.5.1 and should be addressed all together in a separate ticket.

comment:12 in reply to: ↑ 8 @SergeyBiryukov2 years ago

Replying to helen:

Would be good to know if that's RTL-specific or not.

It is. LTR looks fine: 22935.media-rtl.2.png. Same window height, RTL: 22935.media-rtl.3.png.

comment:13 @helen2 years ago

  • Summary changed from RTL media to Horizontal scrollbar in RTL media modal

comment:14 @nacin2 years ago

  • Keywords has-patch needs-refresh added

So, needs a new patch?

comment:15 @SergeyBiryukov2 years ago

  • Keywords needs-patch added; has-patch needs-refresh removed

comment:16 @nacin2 years ago

  • Milestone changed from 3.5.1 to 3.6

Not serious, no patch, moving to 3.6.

@lessbloat2 years ago

comment:17 @lessbloat2 years ago

  • Keywords needs-patch removed

22935.1.diff​ should do the trick.

comment:18 @lessbloat2 years ago

  • Keywords has-patch added

comment:19 @alex-ye23 months ago

  • Cc nashwan.doaqan@… added

comment:20 @nacin21 months ago

  • Keywords rtl-feedback commit added

22935.1.diff​ should do the trick.

Let's double-check this with an RTL language and then we're good to go.

@ocean9021 months ago

comment:21 @ocean9021 months ago

22935.1.diff removes the vertical centering. 22935.2.patch adds it back.

It also removes the part for the webkit search. This should be done in another ticket, but for me this should be done by the browser itself. IE 10 does it.

comment:22 @nacin21 months ago

Is margin-top: 10% sufficient to add back vertical centering? Not entirely sure what's going on here.

I didn't realize this patch wasn't RTL-specific in its changes, otherwise I wouldn't have marked it for commit so easily.

comment:23 @helen21 months ago

  • Keywords commit removed
  • Milestone changed from 3.6 to Awaiting Review

I don't understand what's going on here, either. 22935.2.patch doesn't fix it for me - in fact, I can't reproduce a horizontal scrollbar anymore without the patch. I did get one with the patch just once, but then couldn't reproduce that, either. Punting, because this is not serious and we seem to be having problems reliably reproducing.

For future reference, because I had forgotten, you need to have an empty media library and be on the Media Library tab.

comment:24 @nacin14 months ago

  • Component changed from RTL to Media
  • Focuses rtl added

comment:25 @nacin14 months ago

  • Keywords reporter-feedback needs-testing added; rtl-feedback removed

comment:26 @ocean907 months ago

  • Keywords needs-patch added; has-patch reporter-feedback needs-testing removed

This is still an issue, same for the attachment browser, see 22935.attachment-browser.png.

Seems to be related to #28010, #27629 and #29352.

Note: See TracTickets for help on using tickets.