Closed Bug 1607432 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: XUL, defect, P2)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1607575

People

(Reporter: mkmelin, Assigned: bytesized)

References

(Regression)

Details

(Keywords: regression)

"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?

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.

Flags: needinfo?(ksteuber)

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=

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

Flags: needinfo?(ksteuber)

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

Assignee: nobody → ksteuber

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.

Depends on: 1607575

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(bgrinstead)
Flags: needinfo?(bgrinstead)
Priority: -- → P2

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?

Flags: needinfo?(mkmelin+mozilla)

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.

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(mkmelin+mozilla)
Resolution: --- → DUPLICATE
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.