GNOME Bugzilla – Bug 694451
Containers shouldn't special case RTL positions
Last modified: 2018-01-17 04:01:25 UTC
During this development cycle, RTL capabilities were added to our CSS machinery; an RTL widget now has a :dir(rtl) pseudoclass attached to it, which is good. In the expectation that themes would use this to rework their structural pseudo-classes selectors for this, the reversal of children style information list in GTK containers in the RTL case was removed too. I don't think this is a great idea, for two reasons: - it makes the theme much bigger and harder to write. Structural pseudo classes are quite tricky to use already. - themes use the visible order everywhere else. When talking about borders and rounded corners it's much easier to think of the absolute position of the children in its sibling list instead (it's the first visible element, so I have to make left corners round) rather than considering both that and the mirrored position for the RTL case. For these reasons, I propose to keep the :dir(rtl) pseudo-classes (which are very useful) but to revert the commits that removed reversal of children style information from containers.
Created attachment 237183 [details] [review] Revert "combobox: Don't special-case RTL child positions anymore" This reverts commit cf712c462d766e32840da21a67708bbf2cbb25a6.
Created attachment 237184 [details] [review] Revert "toolbar: Don't special-case RTL toolbar child positions anymore" This reverts commit 821a675013e04598503d3c5ea23ab91607f98e70.
Created attachment 237185 [details] [review] Revert "box: Don't special-case RTL hbox child positions anymore" This reverts commit 6f86e57c4fb2cd76549910302b3a7145e7fd0e8b.
There's a bunch of reasons why I removed this: (1) It's how the web works - your document structure doesn't change when you switch direction. (2) The code was subtly buggy in some of those patches. So it's hard to write this code correctly. (3) It requires even more hoops in the code when transitioning to actors. Because what does box.get_actor().get_first_child() give you?
(In reply to comment #4) > There's a bunch of reasons why I removed this: > > (1) It's how the web works - your document structure doesn't change when you > switch direction. But that's what GTK does already, right? Everything changes in the GTK layout when the direction is changed, so I think it makes sense for the style hierarchy information to change too. > (2) The code was subtly buggy in some of those patches. So it's hard to write > this code correctly. Ah, where was it buggy? I agree this kind of code is too hard to write correctly. > (3) It requires even more hoops in the code when transitioning to actors. > Because what does box.get_actor().get_first_child() give you? As far as I understand these patches wouldn't change the result of that call; they only allow you to assume the style information for children of a container is in the same order as the children themselves - which can be useful too I imagine. In any case, the actors work is not going to happen for 3.8, and without quite some effort spent on the theme at this point, RTL for GTK apps would be broken in GNOME 3.8. I am not sure I can commit to the work of updating the theme before 3.8 is released. I propose to revert these patches for 3.8 (possibly fix the bug you mention) and reconsider them when actors will be introduced. What do you think?
(In reply to comment #5) > I propose to revert these patches for 3.8 (possibly fix the bug you mention) > and reconsider them when actors will be introduced. What do you think? > Sounds like the best way forward.
(In reply to comment #6) > (In reply to comment #5) > > I propose to revert these patches for 3.8 (possibly fix the bug you mention) > > and reconsider them when actors will be introduced. What do you think? > > > Sounds like the best way forward. Cool! I now pushed these to git master - retitling the bug so we can change the behavior again next cycle.
I assume this is no longer relevant.