-moz-box-ordinal-group doesn't preserve secondary sort (by content order) under mutation

NEW
Unassigned

Status

()

Core
Layout
7 years ago
4 years ago

People

(Reporter: dbaron, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
This is a followup to bug 555987.

Handling of dynamic changes to -moz-box-ordinal-group (and the XUL ordinal attribute) now correctly handles the primary sort key (the ordinal number) following mutation of the ordinals.  However, it does not handle the secondary sort key (sort by content order) correctly, since, following a mutation, the code does not re-sort by content order (only by ordinal); instead it inserts the element with the changed ordinal after its previous sibling and then does a sort by ordinal that is otherwise a stable sort.

I believe this is why the reftest layout/reftests/box-ordinal/dynamic-1-remove-to-one-grouped-2.xul fails.

Additionally, the tests in layout/reftests/box-ordinal/ are only testing very simple cases and probably ought to be expanded a bit.
(Reporter)

Comment 1

7 years ago
In bug 555987 comment 10, I wrote:
> We could make nsLayoutUtils::CompareTreePosition part of the sort algorithm.
...
> (We could also make ordinal changes a separate style hint so that we almost
> never have to deal with handling them, which would also make them not require a
> reframe.)
Also see bug 102054, which has now been fixed besides for this...

Updated

5 years ago
Blocks: 467520

Comment 3

5 years ago
Created attachment 637063 [details]
Testcase

Steps to reproduce:

1. Open testcase, observe "div1" text being displayed before "div2".
2. Click "Remove div1 and insert it into the same place" button.
3. Use Inspector tool or something comparable to verify that step 2 didn't change the DOM - the element was put into the same place where it was before.

Expected results:

Display doesn't change, div1 is still displayed before div2 (it works this way in Chrome 20).

Actual results:

The elements change positions, now div2 is displayed before div1 (reproduced in Firefox 13.0.1 and 16.0a1 nightly). This is due to an invisible element with -moz-box-ordinal-group CSS property, without this element things work correctly.

This affects Adblock Plus because the add-on bar in Firefox has the status bar as one of its children - and it has an orient="1000" attribute. So Adblock Plus will insert its icon into the correct position (according to user's customization) but it might be displayed elsewhere.

Comment 4

5 years ago
Maybe this is helpful:

before the update to Adblock Plus 2.1 I had the icons:

About Info - Separator - Firebug - Adblock Plus - Separator - other icons (FoxClocks, Rikaichan, Enable/Disable Flash from the QuickJava addon)

after the update I had:

About Info - Separator - Separator - Firebug - other icons - Adblock Plus

Not only moving the Adblock Plus icon back to its original position failed, also to move the Firebug icon in between the 2 Separators didn't work (I then removed one of the 2 separators).

Maybe also worth mentioning that I have the QuickJava and the Toolbar Buttons addons installed, which provide additional icons which are not available in Firefox out of the box.

Comment 5

5 years ago
Can a developer please look at this bug?
Thank you!

More info here:
https://adblockplus.org/forum/viewtopic.php?f=11&t=10493&p=60997

Comment 6

4 years ago
This affects session manager as well now that it has gone restartless.
You need to log in before you can comment on or make changes to this bug.