Right now isContentEditable and related functions forces style recalc just to check the computed value of -wekbit-user-modify but we shouldn't have to on most documents as -webkit-user-modify CSS property is almost never used in practice. On documents that don't use -webkit-user-modify, we should be able to simply traverse up the DOM tree and find the first node with contenteditable content attribute. We could even have some flag on Node if we wanted to make it super fast.
Created attachment 247316 [details] WIP
<rdar://problem/19949863>
Created attachment 247321 [details] WIP2
Created attachment 247323 [details] Whole WIP2
Comment on attachment 247323 [details] Whole WIP2 View in context: https://bugs.webkit.org/attachment.cgi?id=247323&action=review > Source/WebCore/dom/DocumentStyleSheetCollection.cpp:452 > + for (auto sheet: m_activeAuthorStyleSheets) { Space before : > Source/WebCore/dom/Node.cpp:558 > +static inline bool determineEditabilityIndependentOfStyle(const Node& node, bool isEditable) You probably want bool& isEditable > Source/WebCore/editing/htmlediting.cpp:156 > + switch (updateStyle) { > + case UpdateStyle: > + if (document.needsStyleRecalc() && !document.styleSheetCollection().usesStyleBasedEditability() && editableType == ContentIsEditable) > + return node->isEditableIgnoringStyle(); > + document.updateStyleIfNeeded(); > + break; I prefer the common case (==no editability defined in style) to be the function body and exceptions early returns. No need to call updateStyleIfNeeded if needsStyleRecalc is already tested false.
Comment on attachment 247323 [details] Whole WIP2 View in context: https://bugs.webkit.org/attachment.cgi?id=247323&action=review >> Source/WebCore/dom/DocumentStyleSheetCollection.cpp:452 >> + for (auto sheet: m_activeAuthorStyleSheets) { > > Space before : Also auto& to avoid ref-counting churn? >> Source/WebCore/dom/Node.cpp:558 >> +static inline bool determineEditabilityIndependentOfStyle(const Node& node, bool isEditable) > > You probably want bool& isEditable Good catch :) > Source/WebCore/dom/Node.cpp:590 > + if (node->isElementNode()) is<Element>(*node) > Source/WebCore/dom/Node.cpp:591 > + return downcast<Element>(node)->matchesReadWritePseudoClass(); for consistency: return downcast<Element>(*node).matchesReadWritePseudoClass();
Comment on attachment 247323 [details] Whole WIP2 View in context: https://bugs.webkit.org/attachment.cgi?id=247323&action=review >> Source/WebCore/editing/htmlediting.cpp:156 >> + break; > > I prefer the common case (==no editability defined in style) to be the function body and exceptions early returns. > > No need to call updateStyleIfNeeded if needsStyleRecalc is already tested false. Even in the case editability is not affected by style, we would still like to use render style to compute the editablity if the style tree is up to date for a better performance so I'd expect the common case to be the one that uses the render tree.
Created attachment 247595 [details] Fixes the bug
Created attachment 247596 [details] Fixes the bug
Attachment 247596 [details] did not pass style-queue: ERROR: Source/WebCore/css/StyleSheetContents.h:164: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 1 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 247597 [details] Updated for ToT
Attachment 247597 [details] did not pass style-queue: ERROR: Source/WebCore/css/StyleSheetContents.h:164: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 247597 [details] Updated for ToT r=me
Comment on attachment 247597 [details] Updated for ToT Clearing flags on attachment: 247597 Committed r180867: <http://trac.webkit.org/changeset/180867>
All reviewed patches have been landed. Closing bug.
This broke several tests on Windows: editing/execCommand/5142012-1.html editing/execCommand/nsresponder-outdent.html editing/inserting/insert-at-end-02.html https://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r180867%20(50050)/results.html Perhaps these just need updated results?
(In reply to comment #16) > This broke several tests on Windows: > > editing/execCommand/5142012-1.html > editing/execCommand/nsresponder-outdent.html > editing/inserting/insert-at-end-02.html > > https://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/ > r180867%20(50050)/results.html > > Perhaps these just need updated results? Yes, they need to be rebaselined on all other platforms. I've been waiting for bots to catch up so that I can do it.
Rebaselined tests in r180870.