Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5137457368530944: Issue 1668 - Fixed direction on add-on pages for right-to-left scripts (Closed)

Created:
Dec. 8, 2014, 7:04 p.m. by Sebastian Noack
Modified:
Dec. 10, 2014, 8:24 a.m.
Visibility:
Public.

Patch Set 1 #

Total comments: 4

Patch Set 2 : Used -(webkit|moz)-border-start #

Total comments: 12

Patch Set 3 : Also consider Persian #

Patch Set 4 : Use browser APIs do determine direction #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -11 lines) Patch
M chrome/content/ui/firstRun.js View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/content/ui/i18n.js View 1 2 3 2 chunks +9 lines, -5 lines 0 comments Download
M chrome/skin/firstRun.css View 1 1 chunk +5 lines, -3 lines 0 comments Download

Messages

Total messages: 11
Sebastian Noack
Dec. 8, 2014, 7:06 p.m. (2014-12-08 19:06:24 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css File chrome/skin/firstRun.css (right): http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css#newcode255 chrome/skin/firstRun.css:255: direction: ltr; /* don't mess with left/right border */ ...
Dec. 8, 2014, 8:35 p.m. (2014-12-08 20:35:08 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css File chrome/skin/firstRun.css (right): http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css#newcode255 chrome/skin/firstRun.css:255: direction: ltr; /* don't mess with left/right border */ ...
Dec. 8, 2014, 8:54 p.m. (2014-12-08 20:54:58 UTC) #3
Sebastian Noack
http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css File chrome/skin/firstRun.css (right): http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css#newcode255 chrome/skin/firstRun.css:255: direction: ltr; /* don't mess with left/right border */ ...
Dec. 8, 2014, 9:22 p.m. (2014-12-08 21:22:59 UTC) #4
Wladimir Palant
LGTM http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css File chrome/skin/firstRun.css (right): http://codereview.adblockplus.org/5137457368530944/diff/5629499534213120/chrome/skin/firstRun.css#newcode255 chrome/skin/firstRun.css:255: direction: ltr; /* don't mess with left/right border ...
Dec. 9, 2014, 8:24 a.m. (2014-12-09 08:24:40 UTC) #5
Thomas Greiner
http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js File chrome/content/ui/i18n.js (right): http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js#newcode24 chrome/content/ui/i18n.js:24: locale = ext.i18n.getMessage("@@ui_locale").replace(/_/g, "-"); What about Safari? We don't ...
Dec. 9, 2014, 10:29 a.m. (2014-12-09 10:29:04 UTC) #6
Sebastian Noack
http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js File chrome/content/ui/i18n.js (right): http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js#newcode24 chrome/content/ui/i18n.js:24: locale = ext.i18n.getMessage("@@ui_locale").replace(/_/g, "-"); On 2014/12/09 10:29:04, Thomas Greiner ...
Dec. 9, 2014, 10:41 a.m. (2014-12-09 10:41:12 UTC) #7
Thomas Greiner
http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js File chrome/content/ui/i18n.js (right): http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js#newcode24 chrome/content/ui/i18n.js:24: locale = ext.i18n.getMessage("@@ui_locale").replace(/_/g, "-"); On 2014/12/09 10:41:13, Sebastian Noack ...
Dec. 9, 2014, 4:51 p.m. (2014-12-09 16:51:56 UTC) #8
Wladimir Palant
http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js File chrome/content/ui/i18n.js (right): http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js#newcode154 chrome/content/ui/i18n.js:154: if (locale == "ar" || locale == "he") On ...
Dec. 9, 2014, 4:59 p.m. (2014-12-09 16:59:08 UTC) #9
Sebastian Noack
http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js File chrome/content/ui/i18n.js (right): http://codereview.adblockplus.org/5137457368530944/diff/5724160613416960/chrome/content/ui/i18n.js#newcode24 chrome/content/ui/i18n.js:24: locale = ext.i18n.getMessage("@@ui_locale").replace(/_/g, "-"); On 2014/12/09 16:51:56, Thomas Greiner ...
Dec. 9, 2014, 5:40 p.m. (2014-12-09 17:40:19 UTC) #10
Wladimir Palant
Dec. 9, 2014, 6:55 p.m. (2014-12-09 18:55:19 UTC) #11
LGTM

Powered by Google App Engine
This is Rietveld