785131 - nsHTMLEditor::RemoveStyleInside may (still) process the passed-in node even when aChildrenOnly is set

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Isaac Aggrey [:isaac]]

Attachment: Don't process any node when aChildrenOnly is true

See bug 711862 for background.

nsHTMLEditor::RemoveStyleInside appears to still process some nodes when the contextual code implies children should be processed first and no nodes processed afterwards.

With the attached patch, nsHTMLEditor::RemoveStyleInside will now only process children then the node itself only if aChildrenOnly is false (i.e., if it's true, it will simply skip processing the node itself and return NS_OK).

Aside: For clarity, it may even be preferable to have a single conditional checking for aChildrenOnly being false rather than three separate conditionals that include aChildrenOnly, but that's more of a style choice.

Comment 1

[(no longer active)]

Comment on attachment 654683 [details] [diff] [review]
Don't process any node when aChildrenOnly is true

Thanks!

Do you have try server access?  If not, can you please file a bug for that and CC me on it to vouch for you?  Once you get try server access, you should push this patch to the try server to make sure that it doesn't break anything!

Comment 2

[Isaac Aggrey [:isaac]]

Not yet, but I'm on it!

Comment 3

[Mozilla RelEng Bot]

Try run for d992868f83ca is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d992868f83ca
Results (out of 84 total builds):
    success: 74
    warnings: 9
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d992868f83ca

Comment 4

[Isaac Aggrey [:isaac]]

(In reply to Ehsan Akhgari [:ehsan] from comment #1)
> Once you get try server access, you should push this patch to the try 
> server to make sure that it doesn't break anything!

It looks like the try server job is mighty green, so it doesn't like I broke anything!

Comment 5

[Mozilla RelEng Bot]

Try run for d992868f83ca is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d992868f83ca
Results (out of 85 total builds):
    success: 74
    warnings: 9
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d992868f83ca

Comment 6

[(no longer active)]

Thanks again for your patch, and sorry for the delay in landing it.  I need to get better at staying on top of my bugmail!  :-)

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c3f1cd372a

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f9c3f1cd372a