346373 - Changing spellcheck attribute on parent element doesn't resync children

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Peter Kasting]

From bug 339127 comment 55, when the spellcheck attribute on a node changes, all the node's children should resync, not just the node itself.  Otherwise dynamic changes to attributes won't affect inheriting editors "on the fly".

Comment 1

[Peter Kasting]

Attachment: Testcase

This testcase demonstrates the issue.

Comment 2

[Peter Kasting]

Attachment: patch v1

This patch fixes my testcase.

The only tricky bit here is that we should traverse past non-HTML elements to their children during the subtree notification.  This patch should get that right, but I don't have a testcase demonstrating it.

Comment 3

[Peter Kasting]

Requesting blocking since we need to actually implement the spellcheck attribute spec correctly in Fx2.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 234856 [details] [diff] [review]
patch v1

>Index: content/html/content/src/nsGenericHTMLElement.cpp
>+    nsIContent* childContent = content->GetChildAt(i);
>+    if (childContent) {

No need for that check if we're sure the child list cannot mutate.

r=bzbarsky with that

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 234856 [details] [diff] [review]
patch v1

sr=me with bz's comment. But if you're not 100% sure that the DOM can't mutate just keep the extra check. Better safe then sorry...

Comment 6

[Peter Kasting]

Checked in on trunk:

/mozilla/content/html/content/src/nsGenericHTMLElement.cpp 1.668
/mozilla/content/html/content/src/nsGenericHTMLElement.h 1.246

I don't think the DOM can mutate, but I'd rather be defensive.  So I left the check in, but added an assertion that the childContent pointer is non-NULL right before it.

Leaving open until this makes it onto branch.

Comment 7

[Peter Kasting]

Attachment: patch (as committed)

Here's what I actually committed.

Comment 8

[Mike Schroepfer]

Comment on attachment 235333 [details] [diff] [review]
patch (as committed)

a=schrep for drivers

Comment 9

[Peter Kasting]

Checked in on branch.

/mozilla/content/html/content/src/nsGenericHTMLElement.cpp 1.596.2.14
/mozilla/content/html/content/src/nsGenericHTMLElement.h 1.221.6.4