Bug 119536 - Refactor highestEditableRoot to avoid a redundant call to rendererIsEditable
Summary: Refactor highestEditableRoot to avoid a redundant call to rendererIsEditable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-06 21:54 PDT by Vani Hegde
Modified: 2013-08-16 18:30 PDT (History)
5 users (show)

See Also:

Attachments
Patch (2.05 KB, patch)
2013-08-06 21:59 PDT, Vani Hegde 		no flags Details | Formatted Diff | Diff
Cleanup (3.19 KB, patch)
2013-08-16 18:19 PDT, Ryosuke Niwa 		benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Vani Hegde 2013-08-06 21:54:01 PDT 
Minor refactoring in highestEditableRoot to avoid a redundant call to rendererIsEditable
Comment 1 Vani Hegde 2013-08-06 21:59:41 PDT 
Created attachment 208236 [details]
Patch
Comment 2 Ryosuke Niwa 2013-08-10 22:37:30 PDT 
Comment on attachment 208236 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208236&action=review

> Source/WebCore/editing/htmlediting.cpp:123
> +    if (highestRoot->hasTagName(bodyTag))
> +        return highestRoot;
> +
> +    node = highestRoot->parentNode();

Please don't repeat the code. We can rearrange the code inside the loop instead.
Comment 3 Ryosuke Niwa 2013-08-16 17:34:19 PDT 
Landed in https://chromiumcodereview.appspot.com/22288004.
Comment 4 Ryosuke Niwa 2013-08-16 18:19:40 PDT 
Created attachment 208971 [details]
Cleanup
Comment 5 Ryosuke Niwa 2013-08-16 18:23:58 PDT 
Comment on attachment 208971 [details]
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=208971&action=review

> Source/WebCore/WebCore.exp.in:-2255
> -__ZNK7WebCore4Node18rendererIsEditableENS0_13EditableLevelE

Apparently this is still used. Reverting that.
Comment 6 Benjamin Poulain 2013-08-16 18:27:22 PDT 
Comment on attachment 208971 [details]
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=208971&action=review

> Source/WebCore/WebCore.exp.in:-2255
> -__ZNK7WebCore4Node18rendererIsEditableENS0_13EditableLevelE

You can't do that.

> Source/WebCore/editing/htmlediting.cpp:116
>      Node* highestRoot = editableRootForPosition(position, editableType);

Rename to highestEditableRoot? rootEditableNode?
Comment 7 Ryosuke Niwa 2013-08-16 18:30:20 PDT 
Committed r154212: <http://trac.webkit.org/changeset/154212>