Create documentOrder function, start refactoring to use it instead of Range::compare functions
Created attachment 406686 [details] Patch
Won’t be landing this for another week, but might be ready for review now.
Comment on attachment 406686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406686&action=review > Source/WebCore/dom/Node.h:676 > +enum class PartialOrdering : uint8_t { Less, Equivalent, Greater, Unordered }; This was based on my misunderstanding of std::partial_ordering, which is not an enumeration. The switch statements won’t be possible if I change this to be a class like std::partial_ordering is, but I suspect I should do that so we can eventually switch over to std::partial_ordering with minimal changes.
Created attachment 406755 [details] Patch
I’ll be landing this on Saturday, but would appreciate a review sooner. This is passing all tests on EWS now.
Would love to get this reviewed so I can land it in a couple days.
Comment on attachment 406755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406755&action=review r=me > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1947 > if (isNativeTextControl() && is<HTMLTextFormControlElement>(*node)) The is<HTMLTextFormControlElement>(*node) could be faster if we now leveraged the fact that we already know node is an Element. is<HTMLTextFormControlElement>(*node) does an is<WebCore::Element>(node) check internally when you pass it a Node. > Source/WebCore/dom/Node.h:676 > +class PartialOrdering { Can you explain why we're not using a simple enum? I personally find this class a bit confusing and not very WebKit like (is_eq() for e.g. does not seem to match WebKit coding style).
Comment on attachment 406755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406755&action=review >> Source/WebCore/dom/Node.h:676 >> +class PartialOrdering { > > Can you explain why we're not using a simple enum? I personally find this class a bit confusing and not very WebKit like (is_eq() for e.g. does not seem to match WebKit coding style). Yes, not my design or specific to WebKit. This is out of C++20, with the name changed from std::partial_ordering to WebCore::PartialOrdering (it’s a small subset, but compatible, of the real std::partial_ordering). My goal is to position us to easily switch to std::partial_ordering once we start using C++20. There will be benefits to using the standard class. https://en.cppreference.com/w/cpp/utility/compare/partial_ordering https://en.cppreference.com/w/cpp/utility/compare/named_comparison_functions I used an enumeration in my first cut, then realized that’s not how std::partial_ordering works, and changed it to match.
Comment on attachment 406755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406755&action=review >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1947 >> if (isNativeTextControl() && is<HTMLTextFormControlElement>(*node)) > > The is<HTMLTextFormControlElement>(*node) could be faster if we now leveraged the fact that we already know node is an Element. is<HTMLTextFormControlElement>(*node) does an is<WebCore::Element>(node) check internally when you pass it a Node. Yes, I’ll fix that.
Comment on attachment 406755 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406755&action=review >>> Source/WebCore/dom/Node.h:676 >>> +class PartialOrdering { >> >> Can you explain why we're not using a simple enum? I personally find this class a bit confusing and not very WebKit like (is_eq() for e.g. does not seem to match WebKit coding style). > > Yes, not my design or specific to WebKit. > > This is out of C++20, with the name changed from std::partial_ordering to WebCore::PartialOrdering (it’s a small subset, but compatible, of the real std::partial_ordering). My goal is to position us to easily switch to std::partial_ordering once we start using C++20. There will be benefits to using the standard class. > > https://en.cppreference.com/w/cpp/utility/compare/partial_ordering > https://en.cppreference.com/w/cpp/utility/compare/named_comparison_functions > > I used an enumeration in my first cut, then realized that’s not how std::partial_ordering works, and changed it to match. Maybe I need to add a comment making this clear?
Committed r266026: <https://trac.webkit.org/changeset/266026>
<rdar://problem/67617503>