superreview granted: [Bug 85420] Need Bidi UI / Menu commands and keyboard shortcut for controling Page & Input fields (textarea) direction for specific locales (or when bidi.browser.ui is set) : [At #2

neil@parkwaycc.co.uk <neil.parkwaycc.co.uk@myrealbox.com> has granted Asaf
Romano <bugs.mano@mail-central.com>'s request for superreview:
Bug 85420: Need Bidi UI / Menu commands and keyboard shortcut for controling
Page & Input fields (textarea) direction for specific locales (or when
bidi.browser.ui is set)
https://bugzilla.mozilla.org/show_bug.cgi?id=85420

Attachment 160969: Seamonkey patch - see next comment
https://bugzilla.mozilla.org/attachment.cgi?id=160969&action=edit

------- Additional Comments from neil@parkwaycc.co.uk
<neil.parkwaycc.co.uk@myrealbox.com>
sr=me with the nits fixed.

>+  var systemLocale;
>+  try {
>+    var localeService =
Components.classes["@mozilla.org/intl/nslocaleservice;1"]
>+				  
..getService(Components.interfaces.nsILocaleService);
>+    systemLocale =
localeService.getSystemLocale().getCategory("NSILOCALE_CTYPE");
>+  } catch (e) {}
>+
>+  rv = /^(he|ar|syr|fa|ur)-/.test(systemLocale);
Nit: This should all be inside the try block i.e.
try {
  var localService = ...
  var systemLocale = ...
  rv = ...
} catch (e) {}

>+ if (aWindow.document.dir == "ltr")
>+    aWindow.document.dir = "rtl";
>+  else
>+    aWindow.document.dir = "ltr";
You could use ? "rtl" : "ltr" here.

>+var showBiDi;
Nit: Should be var gShowBidi = false; (gShowBidi because of global variable
style and = false because non-navigator windows should have it explicitly set
to false for the context menu).

>+  if (window.getComputedStyle(aElement, "").direction == "ltr")
>+    aElement.dir = "rtl";
>+  else
>+    aElement.dir = "ltr";
Could use ?: here too if it will fit on one line.
0
bugzilla
10/13/2004 9:42:11 AM
netscape.mozilla.reviewers 29156 articles. 0 followers. Follow

0 Replies
636 Views

Similar Articles

[PageSpeed] 56

Reply: