Summary: | Avoid an ancestor walk in HTMLFormControlElement::computeWillValidate() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | New Bugs | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, mifenton, rniwa, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-02-03 13:28:19 PST
Created attachment 419180 [details]
Patch
Comment on attachment 419180 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419180&action=review This is fine but another approach is to create a new node flag like we do for SelfOrAncestorHasDirAuto and set this bit whenever an ancestor has datalist. > Source/WebCore/dom/Document.h:1331 > + void addDisabledFieldsetElement() { ++m_disabledFieldsetElementsCount; } Can we avoid changing the unrelated code? > Source/WebCore/dom/Document.h:1334 > + bool hasDataListElements() const { return m_dataListElementCount; } We should have this in TreeScope instead. Otherwise, we're gonna have a bad time inside a ShadowRoot. > Source/WebCore/dom/Element.cpp:2220 > + auto* page = document().page(); > + if (page) { Please define page inside if. > Source/WebCore/html/HTMLDataListElement.cpp:61 > + if (parentOfInsertedTree.isInTreeScope()) This isn't right. We're checking that the inserted ancestor's new parent is in a tree scope. It doesn't mean it's in the document. Make the change above to move the function to TreeScope and this will be correct but it's better to check insertionType.treeScopeChanged instead even in that case. > Source/WebCore/html/HTMLDataListElement.cpp:70 > + if (oldParentOfRemovedTree.isInTreeScope()) > + document().decrementDataListElementCount(); Ditto. > Source/WebCore/html/HTMLDataListElement.cpp:76 > +void HTMLDataListElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument) > +{ This function override won't be needed once we made all the changes I suggest above. Created attachment 419212 [details]
Patch
Comment on attachment 419212 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419212&action=review > Source/WebCore/ChangeLog:13 > + * dom/Document.h: > + (WebCore::Document::addDisabledFieldsetElement): Update these change log entries? > Source/WebCore/html/HTMLDataListElement.cpp:64 > + if (insertionType.treeScopeChanged) > + treeScope().incrementDataListElementCount(); We can do this before calling HTMLElement::insertedIntoAncestor. > Source/WebCore/page/PointerLockController.cpp:106 > -void PointerLockController::elementRemoved(Element& element) > +void PointerLockController::elementWasRemoved(Element& element) Or maybe didRemoveElement? Created attachment 419225 [details]
Patch
|