468714 - The Persian installer's UI should be RTL.

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[(no longer active)]

Attachment: Make fa installer RTL

Comment 1

[Axel Hecht [:Pike]]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

Mind putting this back into my review queue when we have the unicode installer up? I don't think it makes sense to review this patch before that.

Comment 2

[(no longer active)]

(In reply to comment #1)
> (From update of attachment 352196 [details] [diff] [review])
> Mind putting this back into my review queue when we have the unicode installer
> up? I don't think it makes sense to review this patch before that.

Sure, will do.

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

For what it is worth this patch has my r+ and can be checked in after the Unicode installer lands and the Persian locale files for the installer lands

Comment 4

[(no longer active)]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

Great.  For technical correctness, could you mark r+ on the patch as well?

Comment 5

[Robert Strong (they/them - no direct email)]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

No problem... I know you already know this but please don't check this in until the Unicode installer and the Persian installer locale files are checked in

Comment 6

[(no longer active)]

(In reply to comment #5)
> (From update of attachment 352196 [details] [diff] [review])
> No problem... I know you already know this but please don't check this in until
> the Unicode installer and the Persian installer locale files are checked in

Sure.  Do I need approval for this for 1.9.1 as well?  Patches in the l10n component don't seem to have approval flags enabled...

Comment 7

[Robert Strong (they/them - no direct email)]

Since this touches toolkit code let's move this over to Toolkit -> NSIS Installer so there is a 1.9.1 approval flag for this

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

Drivers, this is a change to toolkit code for the Persian locales installer's RTL setting and is an extremely simple patch.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 352196 [details] [diff] [review]
Make fa installer RTL

a191=beltzner

Comment 10

[(no longer active)]

Checked in on trunk: <http://hg.mozilla.org/mozilla-central/rev/6e089951b915>

Check in on 1.9.1: <http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1d485fd75e5d>

Comment 11

[Robert Strong (they/them - no direct email)]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 352196 [details] [diff] [review] [details])
> > No problem... I know you already know this but please don't check this in until
> > the Unicode installer and the Persian installer locale files are checked in
> 
> Sure.  Do I need approval for this for 1.9.1 as well?  Patches in the l10n
> component don't seem to have approval flags enabled...
I see the locales.nsi check in... has the installer's locale files been checked in?

Comment 12

[(no longer active)]

(In reply to comment #11)
> I see the locales.nsi check in... has the installer's locale files been checked
> in?

Yes:

http://hg.mozilla.org/l10n-central/fa/rev/4b95a0754f59
http://hg.mozilla.org/releases/l10n-mozilla-1.9.1/fa/rev/3a9f1f483dd7

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with the fa 3.5.1 installer.