Bug 159113

Summary: Don't keep all newly created potential custom elements alive when the feature is disabled
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, dbates, esprehn+autocc, kangil.han, kling, koivisto, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fixes the bug dbates: review+

Ryosuke Niwa
Reported 2016-06-24 23:02:32 PDT
Custom elements spec has been updated to not upgrade all elements unless they're inserted into a document. We don't have a time to update the code fully but we should at least avoid queuing them up and keeping them all alive when the feature is disabled.
Attachments
Fixes the bug (1.75 KB, patch)
2016-06-24 23:10 PDT, Ryosuke Niwa
dbates: review+
Ryosuke Niwa
Comment 1 2016-06-24 23:10:29 PDT
Created attachment 282047 [details] Fixes the bug
Daniel Bates
Comment 2 2016-06-26 20:15:37 PDT
Comment on attachment 282047 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=282047&action=review OK > Source/WebCore/ChangeLog:9 > + The custom elements spec has also been updated to not do this but we'll implement that behavior later. Maybe a better way to say this is: Ideally we want to conform to the behavior in the Custom Elements spec. and only upgrade an element that is inserted into the document. > Source/WebCore/dom/Document.cpp:900 > + if (RuntimeEnabledFeatures::sharedFeatures().customElementsEnabled() > + && Document::validateCustomElementName(localName) == CustomElementNameValidationStatus::Valid) { Can we/would it make sense to write a test for this change? I would hope that such a test could be updated once we implement the behavior defined in the spec.
Ryosuke Niwa
Comment 3 2016-06-26 21:14:08 PDT
(In reply to comment #2) > Comment on attachment 282047 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282047&action=review > > > Source/WebCore/dom/Document.cpp:900 > > + if (RuntimeEnabledFeatures::sharedFeatures().customElementsEnabled() > > + && Document::validateCustomElementName(localName) == CustomElementNameValidationStatus::Valid) { > > Can we/would it make sense to write a test for this change? I would hope > that such a test could be updated once we implement the behavior defined in > the spec. It's theoretically possible with GCController.collect() but I've never been able to write a test with that successfully with JSC in the past. The test always ends up being either flaky or not functional.
Radar WebKit Bug Importer
Comment 4 2016-06-27 20:24:55 PDT
Ryosuke Niwa
Comment 5 2016-06-27 20:26:05 PDT
Note You need to log in before you can comment on or make changes to this bug.