WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#13663 closed defect (bug) (fixed)

Upgrade Button in IE7 RTL is unclickable

Reported by: filosofo Owned by: yoavf
Milestone: 3.0 Priority: normal
Severity: major Version: 3.0
Component: RTL Keywords: has-patch
Focuses: Cc:

Description

Will make a patch as soon as I have opportunity.

Attachments (2)

rtl-upgrade-db.png (62.0 KB) - added by filosofo 4 years ago.
13663-ie-rtl-fix.patch (2.6 KB) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (14)

filosofo4 years ago

comment:1 ocean904 years ago

It seems that this is an IE 6 and 7 bug. The margin: X auto doesn't work with RTL.
A solutuion could be:

html {direction:ltr;}
body {direction: rtl;}

See here.

comment:2 nacin4 years ago

  • Keywords needs-rtl needs-patch added

Seems sensible.

comment:3 filosofo4 years ago

I'd like to find a more general solution first. The fewer targeted hacks we have to make the better.

comment:4 azaozz4 years ago

Another common solution to this is adding text-align: center on the main container (needs to be defined in ie.css with higher priority).

comment:5 ocean904 years ago

azaozz: I tried, but it doesn't work. The HTML Output:

<body>
<h1 id="logo"><img alt="WordPress" src="images/wordpress-logo.png" /></h1>


<h2>No Upgrade Required</h2>
<p>Your WordPress database is already up-to-date!</p>
<p class="step"><a class="button" href="http://localhost/wp/">Continue</a></p>

</body>

We need the fix also for the confirm box in multisite and also if we would support RTL in wp_die().
We should load the IE stylesheet, give the upgrade and confirm box a class and use this for the IE hack.

ocean904 years ago

comment:6 ocean904 years ago

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

13663-ie-rtl-fix.patch: This is the simplest solution I think. It breaks nothing but solves the IE 6 and 7 bug.

comment:7 ocean904 years ago

  • Component changed from Administration to RTL
  • Owner set to yoavf

comment:8 azaozz4 years ago

The patch seems to be working properly, just bear in mind this hack will be applied everywhere ie.css is loaded (pretty much on all screens).

comment:9 follow-up: ocean904 years ago

azaozz: It will applied when ie-rtl.css is loaded. And that shouldn't be a problem because <body> is always RTL, the patch only override the direction from <html>.

comment:10 in reply to: ↑ 9 azaozz4 years ago

Replying to ocean90:

azaozz: It will applied when ie-rtl.css is loaded...

Yes, exactly. We load ie.css conditionally on nearly all pages in the admin so ie-rtl.css will be loaded too (in RTL mode). I didn't see problems with the patch applied just wanted to get a confirmation from RTL languages users too.

comment:11 yoavf4 years ago

  • Status changed from new to accepted

Patch looks good.

We already assume that the HTML element is always "LTR" - the only other difference that this patch will make is that it will move the window vertical scrollbars from left to right as well - but that's how I'd want it and how it's currently behaving in other browsers.

comment:12 nacin4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [15146]) Numerous RTL improvements. props ocean90, TECannon. fixes #13663, fixes #13677.

Note: See TracTickets for help on using tickets.