Summary: | REGRESSION(r96257): Deleting a large amount of text is very slow | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adele Peterson <adele> | ||||||
Component: | HTML Editing | Assignee: | Enrica Casucci <enrica> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | adele, enrica, mitz, rniwa, una.sabovic | ||||||
Priority: | P1 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Adele Peterson
2012-04-14 11:56:59 PDT
Created attachment 139113 [details]
Patch
Comment on attachment 139113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139113&action=review Thanks for the fix! > Source/WebCore/ChangeLog:17 > + Reviewed by NOBODY (OOPS!). This line should appear before the long description. > Source/WebCore/dom/Position.cpp:180 > + return minOffsetForNode(m_anchorNode.get(), m_offset); minOffset sounds like it's the minimum offset. Maybe capOffsetInNode? or roundOffsetInNode? > Source/WebCore/dom/Position.h:295 > + for (Node* n = node->firstChild(); n && newOffset < offset; n = n->nextSibling()) Please don't use abbreviations like n. > Source/WebCore/dom/Position.h:307 > + for (Node* n = node->firstChild(); n && currentOffset < offset; n = n->nextSibling()) Ditto. clip, trim, or truncate may also work. > > Source/WebCore/dom/Position.cpp:180 > > + return minOffsetForNode(m_anchorNode.get(), m_offset); > > minOffset sounds like it's the minimum offset. Maybe capOffsetInNode? or roundOffsetInNode? I'm not sure I agree. This is actually the minimum between offset and the last offset in the node). > > > Source/WebCore/dom/Position.h:295 > > + for (Node* n = node->firstChild(); n && newOffset < offset; n = n->nextSibling()) > > Please don't use abbreviations like n. > > > Source/WebCore/dom/Position.h:307 > > + for (Node* n = node->firstChild(); n && currentOffset < offset; n = n->nextSibling()) > > Ditto. You're right, I'll fix it. Thanks for the review. (In reply to comment #4) > > > Source/WebCore/dom/Position.cpp:180 > > > + return minOffsetForNode(m_anchorNode.get(), m_offset); > > > > minOffset sounds like it's the minimum offset. Maybe capOffsetInNode? or roundOffsetInNode? > I'm not sure I agree. This is actually the minimum between offset and the last offset in the node). Okay. I guess it's not as counter-intutive on my second thought. Committed revision 115460. |