Closed Bug 962283 Opened 10 years ago Closed 10 years ago

Lay out Metro "Firefox Start" page right-to-left in RTL locales


(Firefox for Metro Graveyard :: Firefox Start, defect, P2)

Windows 8.1


(Not tracked)

Firefox 30


(Reporter: mbrubeck, Assigned: sfoster)



(Keywords: rtl, Whiteboard: p=3 s=it-30c-29a-28b.1 r=ff30)


(3 files, 2 obsolete files)

The Firefox for Metro home page (about:start) should use a right-to-left layout when running in an RTL locale (see bug 851165 for general RTL info).
Assignee: nobody → sfoster
Target Milestone: --- → Firefox 30
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [defect] p=3 → [defect] p=3 s=it-30c-29a-28b.1
Attached patch Fix RTL rendering of Start page (obsolete) — Splinter Review
This seems to do it. I checked how Window's Start handles the right-to-left selection state and they flip the orange triangle to the left side of the tile, but leave the checkmark facing its normal direction - so I've done the same (hence the new image)

I'm developing and testing on a patch that sets intl.uidirection.en to 'rtl' and also patches the global.dtd. I'll attach that patch here for convenience, as well as a couple of screenshots
Attachment #8371289 - Flags: review?(mbrubeck)
Attached patch Fix RTL rendering of Start page (obsolete) — Splinter Review
Spotted and fixed an issue with the narrow titles (snapped view). Turns out content: ">" will get automatically reversed, so the -moz-locale-dir(rtl) rule we had was having the opposite of the intended effect.
Attachment #8371289 - Attachment is obsolete: true
Attachment #8371289 - Flags: review?(mbrubeck)
Attachment #8371292 - Flags: review?(mbrubeck)
I've noticed that adding dir="rtl" (dir="&locale.dir;") to the <body> of the start page is necessary, but breaks panning of the page. Take it out, panning works again, but the content is only partially RTL-ed. Unless I'm messing something up here with this patch we'll need to file and fix that too.
I've been testing with this patch in my queue. You'll need to build browser/metro as well as dom/locales
Attachment #8371296 - Attachment is patch: true
Attachment #8371296 - Attachment mime type: message/rfc822 → text/plain
Comment on attachment 8371292 [details] [diff] [review]
Fix RTL rendering of Start page

Review of attachment 8371292 [details] [diff] [review]:

This looks good, but it breaks the firstrun experience.  (Test by setting browser.firstrun.count > 0.)  Horizontal scrolling also seems to be disabled on about:start when it has overflowed the screen.  (The overflow is just hidden instead.)

We'll need to fix those issues before landing this, though they can be fixed in separate patches if you want.

::: browser/metro/theme/tiles.css
@@ +93,5 @@
>  }
> +richgriditem:-moz-locale-dir(rtl) .tile-start-container {
> +  right: 10px;
> +  left: 0;
> +}

Nice, the changes in this file also fix bug 965397.  (Might want to mark that as a duplicate.)
Attachment #8371292 - Flags: review?(mbrubeck) → review-
Amended to fix scrolling issues in the start page, and to update the first-run styles to handle right-to-left. I changed the arrow classnames in firstrun.css to be less confusing; arrow-left becomes arrow-back, arrow-right becomes arrow-forward. It had been messing with my head - these labels now remain accurate in either direction.
Attachment #8371292 - Attachment is obsolete: true
Attachment #8372652 - Flags: review?(mbrubeck)
Whiteboard: [defect] p=3 s=it-30c-29a-28b.1 → p=3 s=it-30c-29a-28b.1 r=ff30
Target Milestone: Firefox 30 → ---
Attachment #8372652 - Flags: review?(mbrubeck) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0
Mozilla/5.0 (Windows NT 6.3; rv:30.0) Gecko/20100101 Firefox/30.0

Verified that the Firefox Start Page layout is right-to-left in RTL locales on latest Nightly (build ID: 20140219030203).
I've used the Arabic, Persian and the Hebrew locales for testing.
Also, verified with the en-US build by creating the string intl.uidirection.en and settinng it to "ltr".

Used a machine running Windows 8.1 32 bit and a Microsoft Surface Pro 2 device running 8.1 64bit for testing.
You need to log in before you can comment on or make changes to this bug.