nsHTMLEditor::IsSimpleModifiableNode does not check for _moz_dirty attribute

NEW
Unassigned

Status

()

Core
Editor
5 years ago
5 years ago

People

(Reporter: glazou, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I have found an error in nsHTMLEditor::IsSimpleModifiableNode(): that method looks for !aContent->GetAttrCount() but does not accept the case of an element having only the _moz_dirty attribute.

An immediate effect of that error is the following one:

 - markup is <p><i>foo</i><i>bar</i></p> 
 - user selects the whole paragraph
 - user hits the i button to remove italic
 - user hits back the i button to re-italicize the result

 EXPECTED RESULT: <p><i>foobar</i></p>
 ACTUAL RESULT:   <p><i>foo</i><i>bar</i></p> 

This happens because the first text node containing "foo" is turned into a <i _moz_dirty>foo</foo> and the code then does not accept to append "bar" at the end of that element instead of encapsulating it into a new <i> element...

I suspect there are other places in the editor where we should accept the presence of a lone _moz_dirty instead of looking only at a zeroed attribute count. This is a regression AFAIC, the original editor was always making sure to never count _moz_dirty in attribute counts.
(Reporter)

Comment 1

5 years ago
Created attachment 728740 [details] [diff] [review]
proposed fix
Attachment #728740 - Flags: review?(ehsan)
Comment on attachment 728740 [details] [diff] [review]
proposed fix

Review of attachment 728740 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense, but please add a test for it too, and make sure that it doesn't break any of the existing ones by pushing to the try server.  Thanks!

::: editor/libeditor/html/nsHTMLEditorStyle.cpp
@@ +267,5 @@
>    }
>  
> +  uint32_t attrCount = aContent->GetAttrCount();
> +  bool noAttr = !attrCount
> +                || (attrCount == 1 && aContent->GetAttrNameAt(0)->Equals(nsGkAtoms::mozdirty));

Nit: please put the or at the end of the previous line.
Attachment #728740 - Flags: review?(ehsan) → review+
You need to log in before you can comment on or make changes to this bug.