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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 3 07:51:35 PST 2011


Ryosuke Niwa <rniwa at webkit.org> 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 80675: fix patch 2
https://bugs.webkit.org/attachment.cgi?id=80675&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
(In reply to comment #16)
> (In reply to comment #15)
> > Right, it's quite horrible.  Would you mind removing
Frame::isContentEditable and adding a FIXME to EditorClient::clientIsEditable?
> 
> I will be happy to work on the follow-up patches. How's this plan sound:
> 1. land this patch that fixes the designmode.
> 2. cleanup Frame::isContentEditable;
> 3. Work on 52058 (renaming Node::isContentEditible).
> 4. improve/fix clientIsEditable and other related issues.

Unfortunately, it can't be a followup.	In this patch, you're eliminating the
call to clientIsEditable and breaking the feature.  So unless we completely
unsupport this feature, our implementation becomes inconsistent.  I suggest the
following steps:

1. Land this patch after removing Frame::isContentEditable and fixing its call
sites all in one patch
2. Remove EditorClient::clientIsEditable

And 1 and 2 can't be broken down into smaller pieces.  Alternatively, you can
do:

1. Get rid of Frame::isContentEditable and remove
EditorClient::clientIsEditable
2. Land this patch

But this might be challenging since we seem to rely on Frame::isContentEditable
for design-mode behavior.


More information about the webkit-reviews mailing list