1509982 - Port Bug 1503824 Migrate the treecols binding into a custom element

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Jorg K (CEST = GMT+2)]

As per bug 1503824 comment #10 (quoted here):

Just a heads up that we are planning to remove the .tree-scrollable-columns wrapping hbox [0] so we can do the CE conversion on treecols without using Shadow DOM (and having to redo styles for treecolpicker).

As far as we can tell, this feature isn't used in Firefox, but I do see one place in TB [1] where there are styles set on that element - presumably you are expecting horizontally scrollable columns in that case? If so, you could probably just move that wrapping hbox into the tree's light DOM around the <treecol> elements when it's used in the document. You'll also need to restore the CSS rule in xul.css for that element.

[0]: https://searchfox.org/mozilla-central/rev/d35199ef15ffa57d190886e20d5df6471faf0870/toolkit/content/widgets/tree.xml#889
[1]: https://searchfox.org/comm-central/search?q=tree-scrollable&path=

Comment 1

[Richard Marti (:Paenglab)]

I asked Philipp in IRC if this tree-scrollable is still used. I think it's no more used.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1509982_tree-scrollable-columns.patch

I think this is all that's needed.

Long calendar names do not need scrolling in the horizontal direction. The name just gets cropped.

Comment 3

[Magnus Melin [:mkmelin]]

Or well, with this there's a slight change in that the row-highlighting doesn't start at the left window edge.

Comment 4

[Richard Marti (:Paenglab)]

Maybe 

calendar-list-tree treechildren::-moz-tree-row {
  padding-inline-start: 18px;
}

does it.

Comment 5

[Magnus Melin [:mkmelin]]

Unfortunately not, you don't get the indention then.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1509982-tree-scrollable-columns.patch

This works for me.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9027819 [details] [diff] [review]
1509982-tree-scrollable-columns.patch

Review of attachment 9027819 [details] [diff] [review]:
-----------------------------------------------------------------

This still doesn't indent the tree children

Comment 8

[Magnus Melin [:mkmelin]]

Or did you try with some other patch applied?

Comment 9

[Richard Marti (:Paenglab)]

On Win10 it does with the m-c patches for the treecols and treecolpicker.

Comment 10

[Richard Marti (:Paenglab)]

On Ubuntu it works too with the m-c patches applied.

Comment 11

[Magnus Melin [:mkmelin]]

Ok, I didn't try with the m-c patches applied

Comment 12

[Richard Marti (:Paenglab)]

Yes, without the m-c patches it doesn't indent.

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 9027819 [details] [diff] [review]
1509982-tree-scrollable-columns.patch

For how long do you want to be busted while waiting for review?

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 9027819 [details] [diff] [review]
1509982-tree-scrollable-columns.patch

Review of attachment 9027819 [details] [diff] [review]:
-----------------------------------------------------------------

I wouldn't say bustage, it would be a smaller visual glitch.

Comment 15

[[:MakeMyDay]]

Comment on attachment 9027819 [details] [diff] [review]
1509982-tree-scrollable-columns.patch

Review of attachment 9027819 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks good.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3fb7643137ce
Port Bug 1503824: adjust CSS as .tree-scrollable-columns went away. r=MakeMyDay DONTBUILD