Closed Bug 902596 Opened 9 years ago Closed 6 years ago

RTL Support for about:networking

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox53 --- verified

People

(Reporter: valentin, Assigned: tomer)

References

Details

(Keywords: rtl, Whiteboard: [necko-would-take])

Attachments

(3 files, 2 obsolete files)

OS: Mac OS X → All
Hardware: x86 → All
Attached patch Full page RTL (obsolete) — Splinter Review
Adding dir=&locale.dir; should be enough, as the page already links to chrome://global/locale/global.dtd.
Attached patch RTL only the warning section (obsolete) — Splinter Review
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]
Attachment #8590792 - Attachment is obsolete: true
Attachment #8590793 - Attachment is obsolete: true
Attachment #8817241 - Flags: review?(mcmanus) → review?(valentin.gosu)
Attachment #8817241 - Flags: review?(valentin.gosu) → review?(jaws)
Keywords: rtl
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED
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+
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.
Assignee: valentin.gosu → tomer.moz.bugs
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dcc8c6ec5de1
RTL Support for about:networking r=jaws
https://hg.mozilla.org/mozilla-central/rev/dcc8c6ec5de1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1334964
QA Whiteboard: [good first verify]
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
You need to log in before you can comment on or make changes to this bug.