corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 677752 - [contentEditable] indent and justify* fail on editable nodes that have only one child; r=ehsan

Browse Source 
Issue #1: indent/justify* can create non-valid fragments.
When applying a block-level formatting to a text node, Gecko creates a div or
blockquote block around the text node and sets the corresponding "align" or
"style" attribute. This patch checks that the active editing host can contain
such a block-level element.

Issue #2: indent/justify* can modify the active editing host.
On the first child of the editable element, the selection is extended outside of
the active editing host -- which causes a few issues for our test cases.
In this patch, this issue is "solved" by modifying
`nsHTMLEditRules::GetPromotedPoint' for block-level operations.


** About the tests **

Sorry for the long explanation but I prefer to be as sharp as possible when I
have to modify existing unit tests.

This patch raises 34 unit test "failures" which are improvements.
Two test files are concerned and have been modified accordingly:
  * test_htmleditor_keyevent_handling
  * test_richtext2.html

One test has been clarified (no real modification):
  * test_bug414526.html

Of course, a specific unit test has been added, see `test_bug677752.html'.


** editor/libeditor/html/tests/test_htmleditor_keyevent_handling.html **
Outdenting now works properly, which results in 4 `FAIL'.

  * 7372 ERROR TEST-UNEXPECTED-FAIL
      | non-tabbable HTML editor: Shift+Tab after Tab on UL
      - got "<ul><li id=\"target\">ul list item</li></ul>",
      expected "<ul><ul><li id=\"target\">ul list item</li></ul></ul>"

  * 7379 ERROR TEST-UNEXPECTED-FAIL
      | non-tabbable HTML editor: Shift+Tab on UL
      - got "ul list item",
      expected "<ul><li id=\"target\">ul list item</li></ul>"

  * 7415 ERROR TEST-UNEXPECTED-FAIL
      | non-tabbable HTML editor: Shift+Tab after Tab on OL -
      got "<ol><li id=\"target\">ol list item</li></ol>",
      expected "<ol><ol><li id=\"target\">ol list item</li></ol></ol>"

  * 7422 ERROR TEST-UNEXPECTED-FAIL
      | non-tabbable HTML editor: Shfit+Tab on OL
      - got "ol list item",
      expected "<ol><li id=\"target\">ol list item</li></ol>"


** editor/libeditor/html/tests/browserscope/test_richtext2.html **
The 15 tests that now pass result in 15 `FAIL' and 15 `UNEXPECTED_PASS'.
Here's an overview of what we had before the patch:

  * Section A - Apply Formatting Tests: +10 points
                    before patch: 21/31 (Selection:  9/31)
                     after patch: 28/31 (Selection: 12/31)

      FB:BQ_TEXT-1_SI    EXECUTION EXCEPTION
      FB:BQ_TEXT-1_SI    EXECUTION EXCEPTION
      FB:BQ_BR.BR-1_SM   EXECUTION EXCEPTION
      FB:BQ_BR.BR-1_SM   EXECUTION EXCEPTION
      IND_TEXT-1_SI      EXECUTION EXCEPTION
      IND_TEXT-1_SI      EXECUTION EXCEPTION
      JC_TEXT-1_SC       editing host is modified
      JF_TEXT-1_SC       editing host is modified
      JL_TEXT-1_SC       editing host is modified
      JR_TEXT-1_SC       editing host is modified

  * Section AC - Apply Formatting Tests, using styleWithCSS: +5 points
                    before patch:  7/18 (Selection:  5/18)
                     after patch: 12/18 (Selection:  5/18)

      IND_TEXT-1_SI      editing host is modified
      JC_TEXT-1_SC       editing host is modified
      JF_TEXT-1_SC       editing host is modified
      JL_TEXT-1_SC       editing host is modified
      JR_TEXT-1_SC       editing host is modified


** editor/libeditor/html/tests/test_bug414526.html **
This test has been clarified to get more explicit report messages -- the test
themselves haven't been changed. A `todo_is' test has been added.
This test is the one that shows that `IsNodeInActiveEditor' can't be modified,
and that limiting the range promotion for block-level operations is preferrable.
This commit is contained in:
Fabien Cazenave
2011-08-23 15:10:14 -04:00
parent 0a723d0d12
commit 76e81b3dc6
6 changed files with 286 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
editor/libeditor/html/nsHTMLEditRules.cpp
View File

@@ -3730,7 +3730,11 @@ nsHTMLEditRules::WillCSSIndent(nsISelection *aSelection, PRBool *aCancel, PRBool
else {
if (!curQuote)
{
// First, check that our element can contain a div.
NS_NAMED_LITERAL_STRING(divquoteType, "div");
if (!mEditor->CanContainTag(curParent, divquoteType))
return NS_OK; // cancelled
res = SplitAsNeeded(&divquoteType, address_of(curParent), &offset);
NS_ENSURE_SUCCESS(res, res);
res = mHTMLEditor->CreateNode(divquoteType, curParent, offset, getter_AddRefs(curQuote));
@@ -3957,6 +3961,10 @@ nsHTMLEditRules::WillHTMLIndent(nsISelection *aSelection, PRBool *aCancel, PRBoo
if (!curQuote)
{
// First, check that our element can contain a blockquote.
if (!mEditor->CanContainTag(curParent, quoteType))
return NS_OK; // cancelled
res = SplitAsNeeded(&quoteType, address_of(curParent), &offset);
NS_ENSURE_SUCCESS(res, res);
res = mHTMLEditor->CreateNode(quoteType, curParent, offset, getter_AddRefs(curQuote));
@@ -4089,10 +4097,10 @@ nsHTMLEditRules::WillOutdent(nsISelection *aSelection, PRBool *aCancel, PRBool *
nsCOMPtr<nsIDOMNode> n = curNode;
nsCOMPtr<nsIDOMNode> tmp;
curBlockQuoteIsIndentedWithCSS = PR_FALSE;
// keep looking up the hierarchy as long as we don't hit the body or a table element
// (other than an entire table)
while (!nsTextEditUtils::IsBody(n) &&
(nsHTMLEditUtils::IsTable(n) || !nsHTMLEditUtils::IsTableElement(n)))
// keep looking up the hierarchy as long as we don't hit the body or the
// active editing host or a table element (other than an entire table)
while (!nsTextEditUtils::IsBody(n) && mHTMLEditor->IsNodeInActiveEditor(n)
&& (nsHTMLEditUtils::IsTable(n) || !nsHTMLEditUtils::IsTableElement(n)))
{
n->GetParentNode(getter_AddRefs(tmp));
if (!tmp) {
@@ -4785,7 +4793,11 @@ nsHTMLEditRules::WillAlign(nsISelection *aSelection,
// or if this node doesn't go in div we used earlier.
if (!curDiv || transitionList[i])
{
// First, check that our element can contain a div.
NS_NAMED_LITERAL_STRING(divType, "div");
if (!mEditor->CanContainTag(curParent, divType))
return NS_OK; // cancelled
res = SplitAsNeeded(&divType, address_of(curParent), &offset);
NS_ENSURE_SUCCESS(res, res);
res = mHTMLEditor->CreateNode(divType, curParent, offset, getter_AddRefs(curDiv));
@@ -4795,9 +4807,9 @@ nsHTMLEditRules::WillAlign(nsISelection *aSelection,
// set up the alignment on the div
nsCOMPtr<nsIDOMElement> divElem = do_QueryInterface(curDiv);
res = AlignBlock(divElem, alignType, PR_TRUE);
// nsAutoString attr(NS_LITERAL_STRING("align"));
// res = mHTMLEditor->SetAttribute(divElem, attr, *alignType);
// NS_ENSURE_SUCCESS(res, res);
//nsAutoString attr(NS_LITERAL_STRING("align"));
//res = mHTMLEditor->SetAttribute(divElem, attr, *alignType);
//NS_ENSURE_SUCCESS(res, res);
// curDiv is now the correct thing to put curNode in
}
@@ -5598,9 +5610,13 @@ nsHTMLEditRules::GetPromotedPoint(RulesEndpoint aWhere, nsIDOMNode *aNode, PRInt
// Don't walk past the editable section. Note that we need to check
// before walking up to a parent because we need to return the parent
// object, so the parent itself might not be in the editable area, but
// it's OK.
if (!mHTMLEditor->IsNodeInActiveEditor(node) &&
!mHTMLEditor->IsNodeInActiveEditor(parent)) {
// it's OK if we're not performing a block-level action.
PRBool blockLevelAction = (actionID == nsHTMLEditor::kOpIndent)
|| (actionID == nsHTMLEditor::kOpOutdent)
|| (actionID == nsHTMLEditor::kOpAlign)
|| (actionID == nsHTMLEditor::kOpMakeBasicBlock);
if (!mHTMLEditor->IsNodeInActiveEditor(parent) &&
(blockLevelAction || !mHTMLEditor->IsNodeInActiveEditor(node))) {
break;
}
@@ -8935,8 +8951,17 @@ nsHTMLEditRules::RelativeChangeIndentationOfElementNode(nsIDOMNode *aNode, PRInt
}
else {
mHTMLEditor->mHTMLCSSUtils->RemoveCSSProperty(element, marginProperty, value, PR_FALSE);
if (nsHTMLEditUtils::IsDiv(aNode)) {
// we deal with a DIV ; let's see if it is useless and if we can remove it
// remove unnecessary DIV blocks:
// we could skip this section but that would cause a FAIL in
// editor/libeditor/html/tests/browserscope/richtext.html, which expects
// to unapply a CSS "indent" (<div style="margin-left: 40px;">) by
// removing the DIV container instead of just removing the CSS property.
nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
if (nsHTMLEditUtils::IsDiv(aNode)
&& (node != mHTMLEditor->GetActiveEditingHost())
&& mHTMLEditor->IsNodeInActiveEditor(aNode)) {
// we deal with an editable DIV;
// let's see if it is useless and if we can remove it
nsCOMPtr<nsIDOMNamedNodeMap> attributeList;
res = element->GetAttributes(getter_AddRefs(attributeList));
NS_ENSURE_SUCCESS(res, res);