RESOLVED FIXED Bug 159113
Don't keep all newly created potential custom elements alive when the feature is disabled
https://bugs.webkit.org/show_bug.cgi?id=159113
Summary Don't keep all newly created potential custom elements alive when the feature...
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.