1585890 - Fix direction of children-pointer in RTL

Status: VERIFIED FIXED

(DevTools :: Memory, defect, P3)

Description

[Itiel]

+++ This bug was initially created as a clone of Bug #1253327 +++

This was apparently fixed in bug 1253327, but for some reason this is still an issue.

Comment 1

[Itiel]

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7ef1e9abd296a8edc39b7efc8d637767ba2f77ed&tochange=1bc2ad020aee2830e0a7941f10958dbec108c254

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:itiel_yn8, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 3

[Julien Wajsberg [:julienw]]

Thanks for the report!
I investigated a bit, and I think the problem is that the transform isn't applied because the element is not a block element.
It's possible that bug 1485363 broke it, but it's not part of the regression window you found, so maybe another thing broke it before.

I think that adding display: inline-block on .children-pointer could work. Do you think you can try this?

Comment 4

[Julien Wajsberg [:julienw]]

Ah, from the regression window, I now think this is regressed by bug 1342672: it removes the "flex" layout. Indeed the flex layout forces a block element for the children, so removing it made it back to inline which is its default.

Comment 5

[Itiel]

(In reply to Julien Wajsberg [:julienw] from comment #3)

Thanks for the report!
I investigated a bit, and I think the problem is that the transform isn't applied because the element is not a block element.
It's possible that bug 1485363 broke it, but it's not part of the regression window you found, so maybe another thing broke it before.

I think that adding display: inline-block on .children-pointer could work. Do you think you can try this?

Thanks for looking into this.
This does work, but also re-introduces the issue mentioned in bug 1253327 comment 10.
To fix it, I changed this:
https://searchfox.org/mozilla-central/rev/deaf58acd27bc663750548e303ac2b461a821cf8/devtools/client/themes/memory.css#408
to padding-right.

This feels kinda hacky, but I don't have a better idea. WDYT?
If that's okay with you I'd like to submit a patch.

Comment 6

[Julien Wajsberg [:julienw]]

Thanks for trying!

I think that's OK to use padding-right in this situation, but please also write a comment nearby explaining why it's OK (that because we use a transform to reverse the icon in RTL, the padding-right gets properly reversed too).

Comment 7

[Itiel]

Attachment: Bug 1585890 - Fix the direction of children-pointer in the Memory inspector's aggregate view for RTL. r?julienw

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2dbdf801599
Fix the direction of children-pointer in the Memory inspector's aggregate view for RTL. r=julienw

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a2dbdf801599

Comment 10

[Itiel]

Fixed on latest Nightly.