RTL Support for about:networking

VERIFIED FIXED in Firefox 53

Status

()

Core
Networking
VERIFIED FIXED
4 years ago
5 months ago

People

(Reporter: valentin, Assigned: tomer)

Tracking

({rtl})

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 verified)

Details

(Whiteboard: [necko-would-take])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 2 obsolete attachments)

Comment hidden (empty)
(Reporter)

Updated

4 years ago
Blocks: 801202
Depends on: 865850
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

2 years ago
Created attachment 8590789 [details]
Screenshot of the [broken] warning screen on Hebrew/RTL build
(Assignee)

Comment 2

2 years ago
Created attachment 8590790 [details]
Screenshot of the [broken] content table on Hebrew/RTL build
(Assignee)

Comment 3

2 years ago
Created attachment 8590792 [details] [diff] [review]
Full page RTL

Adding dir=&locale.dir; should be enough, as the page already links to chrome://global/locale/global.dtd.
(Assignee)

Comment 4

2 years ago
Created attachment 8590793 [details] [diff] [review]
RTL only the warning section

On the other hand, while the warning screen looks bad on RTL, it makes sense to keep the information on the page LTR. I'm keeping here both options for your considerations.
Whiteboard: [necko-would-take]
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8590792 - Attachment is obsolete: true
(Assignee)

Updated

9 months ago
Attachment #8590793 - Attachment is obsolete: true
Attachment #8817241 - Flags: review?(mcmanus) → review?(valentin.gosu)
(Reporter)

Updated

9 months ago
Attachment #8817241 - Flags: review?(valentin.gosu) → review?(jaws)
(Assignee)

Updated

9 months ago
Keywords: rtl
Comment hidden (mozreview-request)
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED

Comment 8

8 months ago
mozreview-review
Comment on attachment 8817241 [details]
Bug 902596 RTL Support for about:networking

https://reviewboard.mozilla.org/r/97616/#review98728

r=me with the following changes made. Thanks for the patch!

::: toolkit/content/aboutNetworking.xhtml:20
(Diff revision 3)
>      <head>
>          <title>&aboutNetworking.title;</title>
>          <link rel="stylesheet" href="chrome://mozapps/skin/aboutNetworking.css" type="text/css" />
>          <script type="application/javascript;version=1.7" src="chrome://global/content/aboutNetworking.js" />
>      </head>
> -    <body id="body">
> +    <body id="body" dir="&locale.dir;">

Please move this attribute to the 'html' node.

::: toolkit/themes/shared/aboutNetworking.css:79
(Diff revision 3)
>    left: -2.3em;
>    top: 0;
>    position: absolute;
>    display: block;
>    width: 1.6em;
>    height: 1.6em;
>    background: url("chrome://global/skin/icons/warning.svg") no-repeat left center;

The 'left' property will need to be updated as well, since the triangle should appear to the right of the title text on RTL builds.

You can write a new ruleset below this one like so,

:root[dir=rtl] .title::before {
  left: auto;
  right: -2.3em;
}

There is no need to change the background-position property.
Attachment #8817241 - Flags: review?(jaws) → review+
(Assignee)

Comment 9

8 months ago
mozreview-review-reply
Comment on attachment 8817241 [details]
Bug 902596 RTL Support for about:networking

https://reviewboard.mozilla.org/r/97616/#review98728

> The 'left' property will need to be updated as well, since the triangle should appear to the right of the title text on RTL builds.
> 
> You can write a new ruleset below this one like so,
> 
> :root[dir=rtl] .title::before {
>   left: auto;
>   right: -2.3em;
> }
> 
> There is no need to change the background-position property.

Isn't we supposed to replace [dir=rtl] with :dir(rtl)?
Yes, please test that :dir(rtl) works as expected and use that. I only tested with [dir=rtl]. Thanks for catching that Tomer.
Comment hidden (mozreview-request)
Assignee: valentin.gosu → tomer.moz.bugs

Comment 12

8 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dcc8c6ec5de1
RTL Support for about:networking r=jaws

Comment 13

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dcc8c6ec5de1
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1334964
QA Whiteboard: [good first verify]

Comment 14

5 months ago
This bug is about RTL support in about:networking page.

I can verify with Latest beta 53.0b8(he) in Ubuntu 16.04 that RTL support is implemented in about:networking.

Build id : 20170330190824
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0

[testday-20170331]
This bug was about "RTL Support for about:networking" and I have seen the feature being implemented with latest Beta(AR) in WIndows 7, 64  Bit !

Build   ID : 20170403072723
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170405]
Thanks!
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
You need to log in before you can comment on or make changes to this bug.