102860 - insertBefore inserts after

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[vva]

Nodes inserted w. the insertBefore() -method are mostly appended and placed 
inconsistently. Applies to mozillas up to the current milestone. See attached 
source and compare w. Internet Explorer 5.5+.

Comment 1

[vva]

Attachment: A sample of the bug. Compare w. IE

Comment 2

[Johnny Stenback (:jst)]

I don't see the problem, please attach a simplified testcase and explain what
the difference is between how Mozilla and IE 5.5+ handle it.

Comment 3

[vva]

Ok, sorry. That's a simplified testcase from something rather more complex I've 
been doing w. IE.

From the interface point of view:

When you click on "section" in the "Insert [ section ]" -link, a new section 
should be created just above and a new "Insert [ section ]" -link above that so 
sections always have one "Insert" -link between them. The sections are numbered 
for illustation putposes. The section w. the next number should appear above 
the "Insert [ section ]" -link you click. Looks like below as it pushes the 
others down, but it is above.

The problem on my 0.9.4 and before that on 0.9.3 is that while IE does just 
that, mozilla appends the section to the end (or right next to another one near 
the end) and the "Insert" -link in different places depending a bit on which 
one I click.

I'll upload a gif (clipboard.gif) to illustate what approximately (can't really 
figure out which is which after a while) the same sequence of clicks produces 
on the two browsers...

If this is due to abug in my code, I apologise for the bother. I've tried this 
with cloning HTML-nodes before I went to creating DOM-nodes. Trying to move the 
sections up and down is even more of a mess...

Comment 4

[vva]

Attachment: ClipBoard.gif - Mozilla vs. IE

Comment 5

[Jason Johnston]

The problem here seems to be that Mozilla is not accurately displaying the
content model. Forcing a refresh of the display (setting .style.display to
'none' then back to 'block') causes it to be correctly drawn.

I've seen this in some of my own work before; I'll try to hack up a simple
testcase that reproduces the bug.

Confirming on win2K build 2001100203.

Comment 6

[Jason Johnston]

Attachment: same as above testcase, with button to refresh display.

Comment 7

[Boris Zbarsky [:bzbarsky]]

The problem lies in the "position: relative", as far as I can tell.  Attaching a
testcase that's much simpler.  (commenting out the position = "relative" part in
the original testcase makes it work, by the way).

Seeing this on current linux trunk.  Is this layout, perhaps?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: simple testcase

Comment 9

[Boris Zbarsky [:bzbarsky]]

Over to layout per Jason's comments.

Comment 10

[Jason Johnston]

Nice call on the position:relative, Boris.  Nice testcase too.  Attaching
another one that shows that this problem is not dependent on the elements being
created from script.

Comment 11

[Jason Johnston]

Attachment: simple testcase, elements not created by script.

Comment 12

[Marc Attinasi]

The content model is correct, but we are not adding the frames correctly
(checked in Viewer). Assigning to Waterson since he is far more adept in the
FrameConstructor than I am (and less swamped too).

Comment 13

[Chris Waterson]

Attachment: fix

This bug was in the code that looked around for prev-in-flow and next-in-flow:
a relatively positioned frame _doesn't_ have a placeholder frame (since it
impacts the normal flow).

Comment 14

[rbs]

Since placeholder frames are created for both IsAbsolutelyPositioned() and 
IsFixePositioned(), is it necessarty to test both too in your changes?

Comment 15

[Chris Waterson]

nsStyleDisplay::IsAbsolutelyPositioned actually checks for either absolute of
fixed positioning. :-/

<http://lxr.mozilla.org/mozilla/source/content/shared/public/nsStyleStruct.h#788>

Comment 16

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 66536 [details] [diff] [review]
fix

r=dbaron

Comment 17

[David Hyatt]

sr=hyatt

Comment 18

[Chris Waterson]

Fix checked in.