427029 - about:robots looks bad on RTL

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Tomer Cohen :tomer]

Attachment: about:robots

the about:robots page have not translated into Hebrew yet, but you can see in the attachment, it require some CSS fixes in order to make it usable (usable?! Are about:robots meant to be useful for something?!).

The image should be positioned in the right side of the page, and maybe even get it mirrored in order to make it face to the text.

Comment 1

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

Comment 2

[Tomer Cohen :tomer]

Attachment: about:robots screenshot (hebrew)

(In reply to bug 437797 comment #10)
> In other words, isn't bug 427029 an edge case given the nature of about:robots
> and it's "optional" status for localizers? Seems like we could just live with
> it not looking perfect for RTL locales that haven't localized it rather than
> adding complexity.
> 

Well, the current state of the page require some attention. If we live with the non-translated page we get it wrong with unnecessary RTL support (see attachment 313609 [details]). But we also get it wrong AFTER translating it. 

I'm attaching another screenshot of how it looks in Firefox 3.0rc2 on Hebrew locale.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> But we also get it wrong AFTER translating it.

What's wrong after translating it? I don't see what the problem is with attachment 324666 [details].

Comment 4

[Tomer Cohen :tomer]

(In reply to comment #3)
> What's wrong after translating it? I don't see what the problem is with
> attachment 324666 [details].
The image should be floated to the other side. Please note the white line (hr) across the image.

Comment 5

[(no longer active)]

Here's the culprit:

<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/aboutRobots.xhtml&rev=1.2&mark=89#88>

We would also need an RTL version of the image as well.

Comment 6

[(no longer active)]

Attachment: RTL version of the robot face (for check-in)

I simply made this by flipping the original image horizontally in GIMP.

Comment 7

[(no longer active)]

Attachment: cvs patch

Patch to respect the RTL mode in about:robots.

Comment 8

[(no longer active)]

Attachment: about:robots screenshot with this patch applied

Comment 9

[(no longer active)]

Given that this has a relatively low-risk patch ready, and it matters to the RTL locales (when for example showing the friends how cool Firefox is in an RTL build), I propose we take this in 3.0.x (3.0.1 if possible).  I don't think, however, that it should block 3.0.1, although having it would be nice.

Comment 10

[Dão Gottwald [:dao]]

If it's just mirrored, I don't think the image needs ui-review. A hg patch would be nice, though.

Comment 11

[(no longer active)]

Attachment: hg patch (for check-in)

Carrying over gavin's r+ from attachment 324765 [details] [diff] [review].  This can be checked in on the trunk together with attachment 324764 [details].

Comment 12

[(no longer active)]

Comment on attachment 324765 [details] [diff] [review]
cvs patch

It would be great if this can land for 1.9.0.2 on branch.  This is a fix for the about:robots page appearance for RTL locales which is pretty low-risk.

Comment 13

[(no longer active)]

Comment on attachment 324764 [details]
RTL version of the robot face (for check-in)

This is needed by attachment 324765 [details] [diff] [review].

Comment 14

[Dão Gottwald [:dao]]

(In reply to comment #11)
> Created an attachment (id=332542) [details]
> hg patch (for check-in)

you can include images in hg patches ...


http://hg.mozilla.org/index.cgi/mozilla-central/rev/f6c0993995c5

Comment 15

[(no longer active)]

(In reply to comment #14)
> you can include images in hg patches ...

Wow, didn't know that!  :-)

Comment 16

[Samuel Sidler (old account; do not CC)]

Comment on attachment 324764 [details]
RTL version of the robot face (for check-in)

Moving approval request out to 1.9.0.3 but I kind of think we shouldn't take this for 1.9.0.x and just pick up the fix in 1.9.1.

Comment 17

[Samuel Sidler (old account; do not CC)]

Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release). And likewise, doesn't meet the criteria for 1.9.0.4.