Closed Bug 785131 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: isaac, Assigned: isaac)

Details

Attachments

(1 file)

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.
Attachment #654683 - Flags: review?(ehsan)
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!
Attachment #654683 - Flags: review?(ehsan) → review+
Not yet, but I'm on it!
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/isaac.aggrey@gmail.com-d992868f83ca
(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!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → isaac.aggrey
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/isaac.aggrey@gmail.com-d992868f83ca
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
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/f9c3f1cd372a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.