141019 - {ib} text appears out of order after pasting or insertion via JS

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[sujay]

using 4/29 trunk build

1) launch netscape
2) launch composer
3) type in some text and new line
4) in browse(separate window), jump to above URL
5) select and copy the bullet: "How to get daily builds"
   in the 3rd column
6) click in the newline in the composer window
7) paste
8) try typing some text after the bullet

results: the text will be on line before the bullet

expected: text to be on line after bullet

Comment 1

[sujay]

marking nsbeta1 and EDITORBASE per Kevin's email.

Comment 2

[Syd Logan]

Sujay, um, what is the URL?

Comment 3

[Syd Logan]

Never mind, I found it.

Comment 4

[Syd Logan]

Taking away the plus, and adding 4xp, and here is why:

The page we are copying from is external content, and we are still trying to fix
problems in content that we generate. Supporting 4.x imported stuff (either by
opening 4.x docs, or copy paste), has to follow the other issues on the list.

In this case, the structure of the list entry is completely different than we
would do in mozilla composer. In mozilla composer, the font specification is
always inside the LI (list item). In 4.x, it is outside. And our engine is not
designed to deal with this easily. 

In the future, we probably want to have some UI that helps us know what we are
importing (via open, or perhaps "paste as Word", "paste as 4.x", etc.. We need
to know what it is we are dealing with an apply an appropriate transformation to
the incoming data to make it compatible with our way of doing things. Not a
suprising thing; many editors have the same issue. We just are not there yet.

Comment 5

[Joe Francis]

content is correct.  This is a layout bug involving font blocks (yuck)

Comment 6

[Joe Francis]

i wuv kin

Comment 7

[Charles Manske]

We need sample list that is not in a behind-NS-firewall document.
Removing minus for Buffy consideration, but I'm inclined to not worry about it
now.

Comment 8

[Kevin McCluskey (gone)]

Kin can not reproduce with current trunk build.

 

Comment 9

[sujay]

updating with new URL..

also I can reproduce with 8/16 build

REOPEN.

Comment 10

[kinmoz]

There instructions above should say "click on one of the blank lines *below* the
list item". I took step 8 above to mean type something in the list item after
the bullet.

Comment 11

[kinmoz]

Attachment: Composer Test Case

Comment 12

[kinmoz]

Attachment: JS Browser Test Case

Comment 13

[kinmoz]

I just attached 2 test cases to this bug. My guess is that we're inserting the
new frame for the text node between the special block-in-inline frames we create
for the <font> tag.

We'll probably have to add some code that takes into account the
NS_FRAME_IS_SPECIAL bit, and keeps getting the next sibling until it finds the
last special frame and insert after it.

Comment 14

[kinmoz]

Attachment: Work in progress patch.

I'm attaching this work in progress patch so I don't lose it while I'm working
on other bugs. This actually fixes the reported test case. But I suspect I'll
have to make the same changes to ContentAppended().

Comment 15

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

See also bug 168883, maybe?

Comment 16

[Kathleen :Brade]

resummarizing since this isn't list-specific

Comment 17

[Kathleen :Brade]

*** Bug 128918 has been marked as a duplicate of this bug. ***

Comment 18

[Kai Engert (:KaiE:)]

I still can reproduce the original reported bug.
Note that there should be a 

  7b) hit enter, 
      or use cursor down,
      or click in the line after the pasted text


The attached patch does not work, it causes the application to crash on Linux,
because symbol "GetSpecialSibling" is undefined.

Comment 19

[Kai Engert (:KaiE:)]

I could fix this "undefined symbol" problem by moving the new function further
down in the source file.

So I have to correct my previous statement:

This patch does have a positive effect on the editor behaviour.
Text that gets typed while the caret is positioned on the line after, will
indeed appear where it is expected.

But unfortunately, this patch does not fix bug 200416.

Comment 20

[Kai Engert (:KaiE:)]

This patch also fixes bug 205528.

The patch was produced 9 months ago. I'd say we should no longer wait but get it in.

Comment 21

[Kai Engert (:KaiE:)]

Comment on attachment 99156 [details] [diff] [review]
Work in progress patch.

David, what do you think (since kin is away currently)? Who else could review
it? Is kin's suggestion reasonable (to change more code), or could we land this
code?

Comment 22

[Kai Engert (:KaiE:)]

Attachment: Updated patch

This patch has no different logic, but it applies correctly to the current
trunk.

Comment 23

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

Comment on attachment 123291 [details] [diff] [review]
Updated patch

This is a bit inefficient, since what you're doing is walking to the
last-in-flow, then (in GetSpecialSibling) walking back to the first-in-flow and
getting its special sibling.  I think it might be better to change
FindPreviousSibling and FindPreviousAnonymousSibling by changing the
GetLastInFlow to if (IsFrameSpecial(...)) { GetLastSpecialSibling} and then
GetLastInFlow(...) -- I suspect the change would be good for the one other
caller.

But this change ought to work...

Comment 24

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

Attachment: patch

This is what I was thinking.

Comment 25

[Kai Engert (:KaiE:)]

I've tested all the testcases in this bug, and the one in bug 205528.
The patch works fine for me.

Comment 26

[Kai Engert (:KaiE:)]

Adding layout peers to cc list.

Could you please review the small patch that kin originally created, and dbaron
rewrote to be more efficient? Thanks!

Comment 27

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

Comment on attachment 123343 [details] [diff] [review]
patch

Requesting 1.4 approval.  This is reasonably low-risk since the current code is
rather badly broken.  It is in core code and does carry some risk, though, but
it fixes a reasonably serious editor bug (and presumably similar bugs with DOM
manipulation from script).

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 123343 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to 1.4

Comment 29

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

Fix checked in to trunk, 2003-05-20 13:49 -0700.

I don't think ContentAppended is an issue (as kin suggested in comment 14) since
it appends to the parent.  (It also should be better tested, since it's used in
page loading.)  So marking bug as fixed.

Comment 30

[Kai Engert (:KaiE:)]

*** Bug 205528 has been marked as a duplicate of this bug. ***