Open
Bug 98778
Opened 23 years ago
Updated 2 years ago
After inserting HR, new text gets inserted at incorrect position (summary in comment #18)
Categories
(Core :: DOM: Editor, defect, P3)
Core
DOM: Editor
Tracking
()
NEW
mozilla1.5beta
People
(Reporter: TucsonTester1, Unassigned)
Details
(Keywords: topembed+, Whiteboard: EDITORBASE+; 2 days <HR>,edt_a3,edt_b3,edt_c3,edt_x3)
Attachments
(1 file)
2.06 KB,
patch
|
mozeditor
:
review-
|
Details | Diff | Splinter Review |
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 3•23 years ago
|
||
-->core Joe--please read/address Daniel's comment above.
Comment 4•23 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•23 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•23 years ago
|
||
-->jfrancis for investigation
Updated•23 years ago
|
Status: NEW → ASSIGNED
Whiteboard: EDITORBASE; 2 days
Updated•23 years ago
|
Whiteboard: EDITORBASE; 2 days → EDITORBASE-; 2 days
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•23 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
minusing
Whiteboard: EDITORBASE; 2 days → EDITORBASE-; 2 days <HR>
Comment 9•23 years ago
|
||
Moving bugs to Mozilla1.1 that are not EDITORBASE+.
Target Milestone: mozilla1.0 → mozilla1.1
Comment 10•22 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•22 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•21 years ago
|
||
editorbase+
QA Contact: sujay → beppe
Whiteboard: EDITORBASE-; 2 days <HR> → EDITORBASE+; 2 days <HR>
Comment 14•21 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.
Updated•21 years ago
|
Whiteboard: EDITORBASE+; 2 days <HR> → EDITORBASE+; 2 days <HR>,edt_a3,edt_b3,edt_c3,edt_x3
Comment 16•21 years ago
|
||
Discussed in bug triage. Can we get a status update and will this make 1.4 final?
Comment 17•21 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•21 years ago
|
Target Milestone: mozilla1.4beta → mozilla1.4final
Comment 18•21 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•21 years ago
|
||
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•21 years ago
|
Attachment #122787 -
Flags: review?(jfrancis)
Comment 20•21 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•21 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•21 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•21 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•21 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•21 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•21 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•21 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
Updated•17 years ago
|
QA Contact: rubydoo123 → editor
Updated•17 years ago
|
Assignee: mozeditor → nobody
Comment 29•6 years ago
|
||
Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•