Fix uses of ContentStatesChanged and MOZ_AUTO_DOC_UPDATE in content/html/content/*.cpp

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla2.0b7
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(1 attachment)

(Assignee)

Comment 1

8 years ago
This has to be fixed before we ship.
blocking2.0: --- → ?

Updated

8 years ago
blocking2.0: ? → betaN+
(Assignee)

Comment 2

8 years ago
Looks like nsGenericHTMLElement.cpp also has some bad uses so let have this bug fixing all of them.
Summary: Fix misuses of ContentStatesChanged and MOZ_AUTO_DOC_UPDATE introduced with :valid/:invalid patch → Fix uses of ContentStatesChanged and MOZ_AUTO_DOC_UPDATE in content/html/content/*.cpp
(Assignee)

Comment 3

8 years ago
Created attachment 471646 [details] [diff] [review]
Patch v1
Attachment #471646 - Flags: review?(bzbarsky)
Comment on attachment 471646 [details] [diff] [review]
Patch v1

>+++ b/content/html/content/src/nsGenericHTMLElement.cpp
> MakeContentDescendantsEditable(nsIContent *aContent, nsIDocument *aDocument)
>+    MOZ_AUTO_DOC_UPDATE(aDocument, UPDATE_CONTENT_STATE, PR_TRUE);

Since this function is recursive, we could end up with tons of update begin/ends here in the common case.  Why not just do a single update around the call in nsGenericHTMLElement::ChangeEditableState?

And maybe file a followup bug to make this iterative instead of recursive?

r=bzbarsky with that.
Attachment #471646 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

8 years ago
I guess I can add MOZ_AUTO_DOC_UPDATE(document, UPDATE_CONTENT_STATE, PR_TRUE); before the initial MakeContentDescendantEditable call. This might be useless if |aDocument && stateBefore != aContent->IntrinsicState()| is always false but I guess it's better than having it called at each recursion step.

I will open a derecursification bug too.
> but I guess it's better than having it called at each recursion step.

Right.  And I would be surprised if it's always false.
(Assignee)

Comment 7

8 years ago
(In reply to comment #5)
> I will open a derecursification bug too.

bug 594472
(Assignee)

Comment 8

8 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/b4cbd426c547
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
You need to log in before you can comment on or make changes to this bug.