Bug 216910

Created attachment 409524 [details] Patch

EWS reported test failures. imported/w3c/web-platform-tests/custom-elements/CustomElementRegistry.html imported/w3c/web-platform-tests/custom-elements/custom-element-registry/define.html WPT has test cases for color-profile element.

We should keep color-profile tag. I'm going to reuse this ticket to add color-profile tag.

Created attachment 409532 [details] Patch

OMG, some test failures and crashes on EWS.

I'm not clear I understand the purpose of this change? What is desired web observable effect you are going for?

My motivations are 1. I'd like to stop using C preprocessor to avoid Cygwin fork failure (Bug 206565 comment 1) 2. I'd like to remove "#if 0" hack which looks too tricky I don't want any web observable effects. Do you have any better idea? Or, should I give up this?

(In reply to Fujii Hironori from comment #7) > My motivations are > 1. I'd like to stop using C preprocessor to avoid Cygwin fork failure (Bug > 206565 comment 1) > 2. I'd like to remove "#if 0" hack which looks too tricky > > I don't want any web observable effects. > Do you have any better idea? Or, should I give up this? Just remove the three lines entirely. Longtime #if 0'd code is not useful.

(In reply to Sam Weinig from comment #8) > (In reply to Fujii Hironori from comment #7) > > My motivations are > > 1. I'd like to stop using C preprocessor to avoid Cygwin fork failure (Bug > > 206565 comment 1) > > 2. I'd like to remove "#if 0" hack which looks too tricky > > > > I don't want any web observable effects. > > Do you have any better idea? Or, should I give up this? > > Just remove the three lines entirely. Longtime #if 0'd code is not useful. Oh, I see, your first change tried that? Hm. I'd like to understand why the #if isn't working right now.

(In reply to Sam Weinig from comment #9) > (In reply to Sam Weinig from comment #8) > > (In reply to Fujii Hironori from comment #7) > > > My motivations are > > > 1. I'd like to stop using C preprocessor to avoid Cygwin fork failure (Bug > > > 206565 comment 1) > > > 2. I'd like to remove "#if 0" hack which looks too tricky > > > > > > I don't want any web observable effects. > > > Do you have any better idea? Or, should I give up this? > > > > Just remove the three lines entirely. Longtime #if 0'd code is not useful. > > Oh, I see, your first change tried that? Hm. I'd like to understand why the > #if isn't working right now. Ok, so make_names.pl does a weird thing where it does two passes over the input, one preprocessed, one not. Very fun. But now I see what we should do. You should go back to your initial change, which is correct, but we need to add "color_profile" to the list of invalid names for custom elements (you can see it in the spec here https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom-element-name). This will probably mean making a change to JSCustomElementRegistry::define(...). If you need some help with this, let me know and I can try my hand at it.

(In reply to Sam Weinig from comment #10) > (In reply to Sam Weinig from comment #9) > > (In reply to Sam Weinig from comment #8) > > > (In reply to Fujii Hironori from comment #7) > > > > My motivations are > > > > 1. I'd like to stop using C preprocessor to avoid Cygwin fork failure (Bug > > > > 206565 comment 1) > > > > 2. I'd like to remove "#if 0" hack which looks too tricky > > > > > > > > I don't want any web observable effects. > > > > Do you have any better idea? Or, should I give up this? > > > > > > Just remove the three lines entirely. Longtime #if 0'd code is not useful. > > > > Oh, I see, your first change tried that? Hm. I'd like to understand why the > > #if isn't working right now. > > Ok, so make_names.pl does a weird thing where it does two passes over the > input, one preprocessed, one not. Very fun. > > But now I see what we should do. > > You should go back to your initial change, which is correct, but we need to > add "color_profile" to the list of invalid names for custom elements (you > can see it in the spec here > https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom- > element-name). This will probably mean making a change to > JSCustomElementRegistry::define(...). If you need some help with this, let > me know and I can try my hand at it. Actually, your original patch was very close to solving this. Instead of removing the line: localName == SVGNames::color_profileTag->localName() just replace it with localName == color_profileLocalName and add: static MainThreadNeverDestroyed<const AtomString> color_profileLocalName("color_profile", AtomString::ConstructFromLiteral); above it.

It sounds great. I'm going to try it. Thanks.

Created attachment 409619 [details] Patch

Committed r267550: <https://trac.webkit.org/changeset/267550> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409619 [details].

<rdar://problem/69529787>