GNOME Bugzilla – Bug 658983
Panel corner highlighting is wrong in RTL locales
Last modified: 2011-09-14 19:27:45 UTC
In RTL locales, the top corners are highlighted when the farthest menu is selected (rather than the nearest). This was caused by the commit http://git.gnome.org/browse/gnome-shell/commit/?id=1ecbabc69a7aff54631d6d0d8c5a61b310fa7824 Here is my attempt at fixing it: diff --git a/js/ui/panel.js b/js/ui/panel.js index 987ca2b..76ab762 100644 --- a/js/ui/panel.js +++ b/js/ui/panel.js @@ -765,9 +765,9 @@ PanelCorner.prototype = { _boxStyleChanged: function() { let button; - if (this._side == St.Side.LEFT) + if (this._side == St.Side.LEFT ^ this.actor.get_direction() == St.TextDirection.RTL) button = this._findLeftmostButton(this._box); - else if (this._side == St.Side.RIGHT) + else if (this._side == St.Side.RIGHT ^ this.actor.get_direction() == St.TextDirection.RTL) button = this._findRightmostButton(this._box); if (button) { I'm not particularly fond of the xor, but it is the shortest way to express this. (And I didn't want to make intrusive changes, but I'd rather rename the functions _find{Leftmost,Rightmost}Button).
Created attachment 196432 [details] [review] panel: Fix corner highlight The style of the top bar's corners is bound to the style of the corresponding button; we used to hardcode this association, but as the login mode does have a different layout, the button is now determined programmatically. Unfortunately, some containers take the text direction into account when ordering their children, while some don't, so the current code returned the wrong button in RTL locales. I noticed that bug a couple of days ago when testing something else(tm), and wrote a quick patch which kept lying in my tree - it's a bit ugly to explicitly special-case St.BoxLayout, but as far as I can tell it is the only container which is RTL-aware ...
Review of attachment 196432 [details] [review]: ::: js/ui/panel.js @@ +774,3 @@ + else if (this._side == St.Side.RIGHT) + side = St.Side.LEFT; + } So, side is undefined if the box isn't a St.BoxLayout, we don't add a button? It would seem much clearer (if that's the intent) to say that explicitly, as in: if (!rtlAwareContainer) return; or something. (Is that the intent?)
Review of attachment 196432 [details] [review]: Nevermind, Jasper set me straight on IRC.
Attachment 196432 [details] pushed as 0518d69 - panel: Fix corner highlight