[webkit-reviews] review denied: [Bug 53031] isContentEditable is not working properly with document.designMode : [Attachment 82050] fix patch 5

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 10 14:52:11 PST 2011


Darin Adler <darin at apple.com> has denied Chang Shu <Chang.Shu at nokia.com>'s
request for review:
Bug 53031: isContentEditable is not working properly with document.designMode
https://bugs.webkit.org/show_bug.cgi?id=53031

Attachment 82050: fix patch 5
https://bugs.webkit.org/attachment.cgi?id=82050&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=82050&action=review

Sorry I didn’t get a chance to look at this before.

> Source/WebCore/dom/Document.cpp:4033
> +    ASSERT(renderer());
> +    if (renderer()) {
> +	   renderer()->style()->setUserModify((value == on) ? READ_WRITE :
READ_ONLY);
> +	   recalcStyle(Inherit);
> +    }

This is not the right way to make style depend on something. This logic should
go into CSSStyleSelector::styleForDocument instead of here. Reaching right in
and tweaking the style isn’t the way to do it.

It’s also wrong to call recalcStyle synchronously. Instead we should be calling
scheduleForcedStyleRecalc here, and calling it only if the new value is
different from m_designMode.


More information about the webkit-reviews mailing list