Closed
Bug 120000
Opened 23 years ago
Closed 23 years ago
Indent list inside table causes table to split
Categories
(SeaMonkey :: Composer, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: selmer, Assigned: mozeditor)
Details
(Whiteboard: EDITORBASE+; fixinhand; need r)
Attachments
(2 files, 1 obsolete file)
9.69 KB,
patch
|
kinmoz
:
superreview+
|
Details | Diff | Splinter Review |
929 bytes,
text/html
|
Details |
1/14 03 trunk build Take a document that has a table inside it. Create several rows and columns within the table. Within one of the cells, create a bullet list and add items to it. At the end of the list, indent the last bullet. Notice that the remainder of the table disappears and the cursor is located in a wacky place.
Updated•23 years ago
|
Keywords: dataloss,
regression
Comment 3•23 years ago
|
||
I can't reproduce this. Please supply a sample page with a table and exact instructions about which cell to add the list to, how many items to add, etc., show this problem.
I just tried this also and I cannot reproduce.. Steve, can you append an attachment of the page you have this problem with? Is it the focal review document ?
Reporter | ||
Comment 5•23 years ago
|
||
It's the focal document. The html in there tends to cause composer fits in general, so maybe this is an edge case. I took the base document and added lists to most of the RHS cells in the topmost accomplishments table before experiencing this.
Steve, I know about the composer fits you are talking about. We've captured most of them due to illegal HTML in these bugs: bug 113310 bug 119565 But we're still trying to reproduce the indent-bullet problem you mention.
Assignee: syd → jfrancis
Assignee | ||
Comment 7•23 years ago
|
||
this document is used with the following test case: In the rightmost column of the table in the Key Accomplishments section (that is the column with "Progress and Accomplishments" in the first row) click in the second row. Then create a list by clicking on the make list button. Type "aaa". Then click indent. This reproduces Steve's problem reliably for me.
Assignee | ||
Comment 8•23 years ago
|
||
099. Note that the rest of the table doest really disappear. It just gets split off. Still a bad bug though.
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.9
Assignee | ||
Comment 9•23 years ago
|
||
attaching patch later tonight (it's on a powerbook thats out of juice and my cord is at home)
Whiteboard: EDITORBASE; fixinhand
Assignee | ||
Comment 10•23 years ago
|
||
I've reworked this patch. There are three problems with indention that are fixed here. Only one of the fixes is strictly needed to solve this problem, but other similar problems could occur without the other fixes. For reviewers: here's what's changed. 1) nsHTMLEditRules::WillHTMLIndent() now skips over non-editable nodes. 2) nsHTMLEditRules::WillHTMLIndent() makes sure there is only a "current list" or "current blockquote" to place indented material. Before there could be both, which was erroneous. 3) nsHTMLEditRules::GetInnerContent() now takes an index, and inserts elements into the supplied nsISupportsArray at the index location, rather than appending. 1) is needed to prevent creating blockquotes or sublists for non-editable content we encounter. 2) is needed to prevent content from being reordered incorrectly in cases where we are blockquoting some nodes and sublisting others. 3) is needed to prevent indented content from becoming reordered when we are acting on multiple table cells or list items. removing dataloss and regression keywords. I don't think this bug was either, though it is pretty nasty.
Keywords: dataloss,
regression
Assignee | ||
Comment 11•23 years ago
|
||
Assignee | ||
Comment 12•23 years ago
|
||
Simplified test case. Open in Composer and try to indent "aaa". Note that if you create a similar looking document in Composer, the bug won't exhibit itself. Bug is exposed by the various non-rendering whitespace formatting nodes in the html source.
Attachment #65431 -
Attachment is obsolete: true
Assignee | ||
Updated•23 years ago
|
Whiteboard: EDITORBASE; fixinhand → EDITORBASE; fixinhand; need r, sr
Assignee | ||
Updated•23 years ago
|
Summary: Data loss: Indent list inside table, lose content → Indent list inside table causes table to split
Comment 13•23 years ago
|
||
Comment on attachment 66974 [details] [diff] [review] patch for editor/libeditor/html/nsHTMLEditRules/cpp The changes look ok to me, with the exception of the logic changes in RemoveEmptyNodes(). I think it should be more like the following, otherwise some of our blocks (p, pre, etc) will always be candidates, even when the selection is in them. This new way also has the added benefit of doing the Is<blocktype>() checks once. Or am I not reading this right? Here's what I think it should be: @@ -6719,12 +6735,10 @@ node = do_QueryInterface(content); if (!node) return NS_ERROR_FAILURE; - PRBool bIsEmptyNode; - res = mHTMLEditor->IsEmptyNode(node, &bIsEmptyNode, PR_FALSE, PR_TRUE); - if (NS_FAILED(res)) return res; + PRBool bIsCandidate = PR_FALSE; // only consider certain nodes to be empty for purposes of removal - if (!( + if ( nsTextEditUtils::NodeIsType(node, NS_LITERAL_STRING("a")) || nsTextEditUtils::NodeIsType(node, NS_LITERAL_STRING("b")) || nsTextEditUtils::NodeIsType(node, NS_LITERAL_STRING("i")) || @@ -6739,18 +6753,13 @@ nsTextEditUtils::NodeIsType(node, NS_LITERAL_STRING("sup")) || nsTextEditUtils::NodeIsType(node, NS_LITERAL_STRING("font")) || nsHTMLEditUtils::IsList(node) || - nsHTMLEditUtils::IsParagraph(node) || - nsHTMLEditUtils::IsHeader(node) || - nsHTMLEditUtils::IsListItem(node) || - nsHTMLEditUtils::IsBlockquote(node)|| nsHTMLEditUtils::IsDiv(node) || - nsHTMLEditUtils::IsPre(node) || - nsHTMLEditUtils::IsAddress(node) ) ) + nsHTMLEditUtils::IsAddress(node) ) { - bIsEmptyNode = PR_FALSE; + bIsCandidate = PR_TRUE; } - if (bIsEmptyNode && !nsTextEditUtils::IsBody(node)) + if (!bIsCandidate && !nsTextEditUtils::IsBody(node)) { if (nsHTMLEditUtils::IsParagraph(node) || nsHTMLEditUtils::IsHeader(node) || sr=kin@netscape.com, with my changes above if they are correct?
Attachment #66974 -
Flags: superreview+
Whiteboard: EDITORBASE; fixinhand; need r, sr → EDITORBASE+; fixinhand; need r, sr
Assignee | ||
Comment 14•23 years ago
|
||
regarding kin's comments: First off, this part of the diff made it in by accident and is part of an unrelated bug fix. But it still needs reviewing eventually, so no effort wasted. I think your changes are an improvement. I don't see the correctness issue you mention: it is true that the was the logic stands now a PRE would lite the candidate flag at one point, but if the selection were in it then the candidate flag would later be unlit. However, your changes do avoid unneccessary testing for certain block types twice. thanks!
Whiteboard: EDITORBASE+; fixinhand; need r, sr → EDITORBASE+; fixinhand; need r
Assignee | ||
Comment 15•23 years ago
|
||
fix landed on trunk
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•