1607432 - can't reorder tree columns (Bookmarks library, Certificate Manager)

Status: RESOLVED DUPLICATE of bug 1607575

(Core :: XUL, defect, P2)

Description

[Magnus Melin [:mkmelin]]

"ordinal" used by the treecol widgets was removed in bug 1603830. The replacement doesn't seem to work. I can no longer reorder tree columns in Bookmarks | Show all Bookmarks nor in the Certificate Manager.

MozTreecol also keeps the setters/getters for ordinal, but doesn't use them. Is this intentional?

Comment 1

[Brian Grinstead [:bgrins]]

I can drag/drop the columns in the cert manager and see them being reordered, along with inline -moz-box-ordinal-group styles being set. But in the Bookmarks manager I'm unable to drag Name to the right, so something does seem wrong here. Kirk, could you please take a look at that?

As mentioned in Comment 0, I think these should change to use style instead of the magic attribute as well, right? https://searchfox.org/mozilla-central/rev/4537228c0a18bc0ebba2eb7f5cbebb6ea9ab211c/toolkit/content/widgets/tree.js#381-393.

Comment 2

[Brian Grinstead [:bgrins]]

There are a few callers of setAttribute("ordinal") in this file which could be causing a problem: https://searchfox.org/mozilla-central/search?q=setAttribute(%22ordina&case=false&regexp=false&path=

Comment 3

[Robin Steuber (she/her) [:bytesized]]

I could have sworn I changed those consumers, but apparently not. I'll take a look.

Comment 4

[Robin Steuber (she/her) [:bytesized]]

Ah, I did change those consumers. I guess searchfox must just not have caught up yet.

Comment 5

[Robin Steuber (she/her) [:bytesized]]

It appears that the remaining consumers of the ordinal attribute overlap with code that has been changed to use el.style.MozBoxOrdinalGroup and they are not playing well together. Ultimately, we want to remove every part of the ordinal attribute including C++ handling and test usage. It looks like there are not very many of consumers, so I plan to do all that work now. However, I think it makes more sense to have a bug dedicated to that, as it is a relatively significant change, if not a particularly difficult one. I will file one now. Fixing that should address this regression.

Comment 6

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

The priority flag is not set for this bug.
:bgrins, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[Robin Steuber (she/her) [:bytesized]]

I believe the issues raised in bug have been fixed by merging Bug 1607575.

@mkmelin Could you check whether things are working as expected for you?

Comment 8

[Magnus Melin [:mkmelin]]

Sorry for the delay. Indeed this is now working. The Thunderbird tests (in bug 1604155) can now run, except for in --headless mode. If you have any ideas why that doesn't work, please comment over there.