Bug 70800 - WebKit nests pre on copy and paste when the pre is the root editable element
Summary: WebKit nests pre on copy and paste when the pre is the root editable element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-24 23:14 PDT by Ryosuke Niwa
Modified: 2011-10-31 16:09 PDT (History)
9 users (show)

See Also:


Attachments
fixes the bug (18.88 KB, patch)
2011-10-29 21:21 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixed typo in comment (18.88 KB, patch)
2011-10-29 21:23 PDT, Ryosuke Niwa
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-24 23:14:56 PDT
Reproduction: <pre contenteditable=true style="width:100%; height:40%; border:solid 1 black; margin-left:4px; padding-left:4px; overflow-y:scroll;"></pre>

http://crbug.com/40480
Comment 1 Ryosuke Niwa 2011-10-29 21:21:18 PDT
Created attachment 112983 [details]
fixes the bug
Comment 2 Ryosuke Niwa 2011-10-29 21:23:21 PDT
Created attachment 112984 [details]
Fixed typo in comment
Comment 3 Darin Adler 2011-10-31 12:05:20 PDT
Comment on attachment 112984 [details]
Fixed typo in comment

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

> Source/WebCore/editing/htmlediting.cpp:1182
> +    return node->hasTagName(listingTag)
> +    || node->hasTagName(olTag)
> +    || node->hasTagName(preTag)
> +    || node->hasTagName(tableTag)
> +    || node->hasTagName(ulTag)
> +    || node->hasTagName(xmpTag)
> +    || node->hasTagName(h1Tag)
> +    || node->hasTagName(h2Tag)
> +    || node->hasTagName(h3Tag)
> +    || node->hasTagName(h4Tag)
> +    || node->hasTagName(h5Tag);

Normally these would all be indented one tab stop (four spaces).

Not sure if the compiler will notice the common subexpression and remove the 11 unneeded checks of the “is element” bit. If not, the function could be written with an early exit and cast to make sure it gets that right.
Comment 4 Ryosuke Niwa 2011-10-31 12:12:36 PDT
Comment on attachment 112984 [details]
Fixed typo in comment

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

Thanks for the review!

>> Source/WebCore/editing/htmlediting.cpp:1182
>> +    || node->hasTagName(h5Tag);
> 
> Normally these would all be indented one tab stop (four spaces).
> 
> Not sure if the compiler will notice the common subexpression and remove the 11 unneeded checks of the “is element” bit. If not, the function could be written with an early exit and cast to make sure it gets that right.

We should just rewrite this using hashmap at some point. Also it's missing h6 :( Will fix the style before landing it.
Comment 5 Darin Adler 2011-10-31 15:39:25 PDT
(In reply to comment #4)
> We should just rewrite this using hashmap at some point. Also it's missing h6 :( Will fix the style before landing it.

This is such a small set of values that a linear or binary search might be faster than a hash map.
Comment 6 Ryosuke Niwa 2011-10-31 16:09:18 PDT
Committed r98899: <http://trac.webkit.org/changeset/98899>