After inserting HR, new text gets inserted at incorrect position (summary in comment #18)

NEW
Unassigned

Status

()

Core
Editor
P2
normal
17 years ago
12 years ago

People

(Reporter: TucsonTester1, Unassigned)

Tracking

({topembed+})

Trunk
mozilla1.5beta
topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: EDITORBASE+; 2 days <HR>,edt_a3,edt_b3,edt_c3,edt_x3)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.3+)
Gecko/20010903 Netscape6/6.1b1
BuildID:    20010903

When you enter in multiple lines with color fonts then add a HRule. You will
notice extra font tags. not needed. 

Reproducible: Always
Steps to Reproduce:
1.Open New Composer Document
2.type 1 line of text, click enter
3.change font color using color pallet from tool bar. 
4.type 1 line of text, click enter
5.select Hrule from toolbar
6.click on source and look at the sorce code 

Actual Results:  You get something that looks like this 
<body>
<font color="#ff0000">Red Text <br>
<font color="#3333ff">Changed to blue Text <br>
</font></font>
<hr width="100%" size="2">  ( <font color="#ff0000"><font color="#3333ff">) <br>
</font><br>
</font>
</body>
The stuff in () is not needed . 

Expected Results:  The font tags should not even be there after the HR tag
Confirming bug.
reporter : right ; the FONT element should not be here if you add no more text

jfrancis : (thinking at loud) could a way of solving this be the following one :
when you insert a HR as last child, you add a BR after it (so the selection works
fine) and you cache html inline styles. In that case, no FONT element is added
until a new char is entered, right ?

brade : I think that it is a WONTFIX only if jfrancis says that what I am
proposing is not possible.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

17 years ago
spam composer change
Component: Editor: Core → Editor: Composer

Comment 3

17 years ago
-->core
Joe--please read/address Daniel's comment above.
Component: Editor: Composer → Editor: Core
Hardware: PC → All

Comment 4

17 years ago
I can't respond until I investigate this bug.  It may be that inserting the HR is 
causing the font tag to be split, due to the DTD.
Target Milestone: --- → mozilla1.0

Updated

17 years ago
Assignee: brade → jfrancis
Summary: Placing a H.Rule after multiple lines of colored text, results in access font tags → Placing a H.Rule after multiple lines of colored text, results in excess font tags

Comment 5

17 years ago
-->jfrancis for investigation

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: EDITORBASE; 2 days

Updated

17 years ago
Whiteboard: EDITORBASE; 2 days → EDITORBASE-; 2 days

Comment 6

17 years ago
The extra tags are because the intent is for the text following the HR to
continue having the same color as the text ahead of the HR. Notice if you follow
the steps but instead insert an image, you get the same color as in the text
above the image, and the "extra" tags that supported this are there. Bug is the
it doesn't work for HR. EDITORBASE.
Summary: Placing a H.Rule after multiple lines of colored text, results in excess font tags → Colors and attributes of text should persist after you enter an HR
Whiteboard: EDITORBASE-; 2 days → EDITORBASE; 2 days

Comment 7

17 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1

Updated

17 years ago
Priority: -- → P2
Target Milestone: mozilla1.0.1 → mozilla1.0

Comment 8

17 years ago
minusing
Whiteboard: EDITORBASE; 2 days → EDITORBASE-; 2 days <HR>
Moving bugs to Mozilla1.1 that are not EDITORBASE+.
Target Milestone: mozilla1.0 → mozilla1.1

Comment 10

16 years ago
The days of having a half dozen milestones out in front of us to divide bugs 
between seem to be gone, though I dont know why.  Lumping everything together as 
far out as I can.  I'll pull back things that I am working on as I go.
Target Milestone: mozilla1.1alpha → mozilla1.2beta

Comment 11

16 years ago
[ushing these out as far as bugzilla will let me.  I'll pull them back as I work
on them.
Target Milestone: mozilla1.2beta → mozilla1.4beta

Comment 12

16 years ago
editorbase+
QA Contact: sujay → beppe
Whiteboard: EDITORBASE-; 2 days <HR> → EDITORBASE+; 2 days <HR>

Comment 13

16 years ago
EDITORBASE+ topembed+ normalization
Keywords: topembed+

Comment 14

16 years ago
using the build 2002031804 on win32, this is still a problem. The resulting HTML
code is this:
<body>
this is one line of text<br>
<span style="color: rgb(51, 51, 255);">this is a line of blue text<br>
</span>
<hr style="width: 100%; height: 2px;">no this is not blue<span
 style="color: rgb(51, 51, 255);"><br>
</span>
</body>

The span gets carried over, however, the focus point is to the left of the span.

Comment 15

16 years ago
-> me
Assignee: jfrancis → kaie
Status: ASSIGNED → NEW

Updated

16 years ago
Whiteboard: EDITORBASE+; 2 days <HR> → EDITORBASE+; 2 days <HR>,edt_a3,edt_b3,edt_c3,edt_x3

Comment 16

15 years ago
Discussed in bug triage.  Can we get a status update and will this make 1.4 final?

Comment 17

15 years ago
I have not yet worked on this bug, but it's on my list of important bugs.
I would like to try to get this into 1.4 final.

Updated

15 years ago
Target Milestone: mozilla1.4beta → mozilla1.4final

Comment 18

15 years ago
Summary:

This bug requests, after inserting a HR, the text style for newly entered text
below the HR, should be the same as above the HR.

The newest trunk code splits the SPAN. This is correct, because a SPAN must not
contain a HR.


For the future talk, let's say the text above the HR is blue, and we assume the
text below to HR to be blue. The bug is: the new text is black.


The interesting thing is: The entered text is only black (incorrect), outside
the SPAN, if you enter the text directly after having inserted the HR.

If you do anything else in between, either move the cursor around, or save and
reopen, the bug does not appear again.


Conclusion:
- the editor produces correct HTML
- the "edit HR" action places the caret at an invalid position


Suggested strategy for fixing the bug:
Either correct the incorrect caret placement code, or, after having inserted a
HR, execute code that ensures the caret is at the correct position.
Summary: Colors and attributes of text should persist after you enter an HR → After inserting HR, new text gets inserted at incorrect position (summary in comment #18)

Comment 19

15 years ago
Created attachment 122787 [details] [diff] [review]
Patch v1

This fixes the problem for me in the reported.
It seems logical to me, the idea is, after inserting an element, find the next
visible thing after the newly inserted element, and place caret in front of
that next element.

The existing code makes the assumption that it will work to simply go to the
next offset+1 position. But that fails in our case, because we insert
additional nodes (split SPAN) into the DOM tree.

Updated

15 years ago
Attachment #122787 - Flags: review?(jfrancis)

Comment 20

15 years ago
I'm going to have to think about this one for a bit, there may be some fallout
from this approach...

Comment 21

15 years ago
20030512 Test Build

Intended behaviour?
1.  Inserting the hrule into a span breaks the span into two spans and moves the
cursor within the span.  
2.  The inserted hrule does not have the same color styling as the text it is
being inserted into. (?)
2.  If cursor is then placed immediately before or after the hrule, text is
entered outside of the spans in the same area as the hrule without any styling. (?)

Comment 22

15 years ago
Some comments below, but I think each of the following suggestions should be
discussed in separate new bugs, since all behaviour you describe can be seen
with trunk builds, too.


> 1.  Inserting the hrule into a span breaks the span into two spans and moves the
> cursor within the span.  

I think that is the intended behaviour, because the sequence <span><hr></span>
seems to be invalid.
Maybe the the <hr> element should obtain the current style.


> 2.  The inserted hrule does not have the same color styling as the text it is
> being inserted into. (?)
> 2.  If cursor is then placed immediately before or after the hrule, text is
> entered outside of the spans in the same area as the hrule without any 
> styling. (?)

I'm not sure how we are supposed to behave in the two cases.

Comment 23

15 years ago
I have some ideas about this.  I did some work on the ANGELON branch earlier
that remembers inline styles when you delete, so that further typing is still in
a style after deleletion.  

This work could be extended to the case where we are inserting a block level
element.  It would keep the insert element code cleaner (it would simply make a
single call to cache the current styles) and it would leave the source cleaner
(we could eliminate empty inline tags that ended up on one side of the inserted
element), and yet it would stll fix the problem.

I think leveraging that other work (which is unfortunately not on the tip) is
the way to go.  The first task is to port the earlier work to the tip.

Comment 24

15 years ago
Joe, the alternate approach you are proposing sounds like a lot of more work.

Do you actually reject the patch I have attached in this bug?

If you do reject it, could you please provide a pointer to the code you are
mentioning, that should get ported, enabling me to do this work?
Thanks.

Moving target milestone, it sounds unlikely this will make it for 1.4 final,
unless Joe accepts the existing patch.

Target Milestone: mozilla1.4final → mozilla1.5beta

Comment 25

15 years ago
Comment on attachment 122787 [details] [diff] [review]
Patch v1

minusing for now because I think &visType needs to be checked to determine
proper way to adjust selection.

But it may be that this whole approach is not the best.  I will talk to Kai
more about this after some investigation.
Attachment #122787 - Flags: review?(jfrancis) → review-

Comment 26

15 years ago
Kai, the code I mentioned earlier has landed.  Try using
nsHTMLEditRules::CacheInlineStyle() to remember the style. Then in
nsHTMLeditRules::AfterEditInner() you will want to find the if clause that
contains ReapplyCachedStyles() and add the appropriate action types to the if
clause.

This should make for a simple fix, and one that keeps the source clean. 

Comment 27

15 years ago
It's not likely that I will work on editor/selection bugs in the near future.
Mass assining my bugs to nobody.

Assignee: kaie → nobody

Comment 28

15 years ago
snarfing kaie's old bugs
Assignee: nobody → mozeditor
QA Contact: rubydoo123 → editor
Assignee: mozeditor → nobody
You need to log in before you can comment on or make changes to this bug.