943751 - RTL Support for pop-ups

Status: RESOLVED INVALID

(Firefox for Android Graveyard :: General, defect)

Description

[u482316]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_4) AppleWebKit/536.30.1 (KHTML, like Gecko) Version/6.0.5 Safari/536.30.1

Steps to reproduce:

Changed the text alignment of various pop-ups so that the text appears to the right instead of the left.

Comment 1

[u482316]

Attachment: edit_top_sites_popup.patch

This patch aligns the user's input text to the right to support right-to-left languages.

The pop-up can be found by going to the top sites page > do a long click on one of the thumbnails > chose the edit option.

Comment 2

[u482316]

Attachment: edit_bookmark_popup.patch

This patch aligns each of the fields in the bookmark pop-up to the right.

To see the pop-up do the following steps:
Go to bookmark page > do a long click on one of the items on the list > click the edit option.

Alternatively, it can also be generated by clicking the menu icon, choose the bookmark option and at the bottom there should be a small pop-up indicating the bookmark has been added and a button labelled "options". Choose options and click edit. (Note that this method works when the sites is not already bookmarked)

Comment 3

[u482316]

Attachment: context_menu_popup.patch

Right align the text on general pop-ups that appear throughout the browser but has not been supported by RTL yet.

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 8339073 [details] [diff] [review]
edit_top_sites_popup.patch

Review of attachment 8339073 [details] [diff] [review]:
-----------------------------------------------------------------

Assuming this attribute is safely ignored on Android versions that don't support it.

Comment 5

[Lucas Rocha (:lucasr)]

Comment on attachment 8339077 [details] [diff] [review]
edit_bookmark_popup.patch

Review of attachment 8339077 [details] [diff] [review]:
-----------------------------------------------------------------

Ditto.

Comment 6

[Lucas Rocha (:lucasr)]

Comment on attachment 8339081 [details] [diff] [review]
context_menu_popup.patch

Review of attachment 8339081 [details] [diff] [review]:
-----------------------------------------------------------------

Is only setting textAlignment on this textview enough for RTL support on prompts?

::: mobile/android/base/prompts/Prompt.java
@@ +633,5 @@
>                  convertView = mInflater.inflate(resourceId, null);
>                  convertView.setMinimumHeight(mMinRowSize);
>  
>                  TextView tv = (TextView) convertView.findViewById(android.R.id.text1);
> +                tv.setTextAlignment(View.TEXT_ALIGNMENT_VIEW_START);

This is going to cause crashes on pre-17 Android. You have to enclose this call with a version check like:

if (Build.VERSION.SDK_INT > 17) {
    tv.setTextAlignment(View.TEXT_ALIGNMENT_VIEW_START);
}

nit: add empty line here.

Comment 7

[Itiel]

Following comment 2's STRon RTL build, the bug seems to be fixed.

Comment 8

[Itiel]

Closing as RESOLVED INVALID.