Bug 229929 - Implement HTMLScriptElement.supports(type) method
Summary: Implement HTMLScriptElement.supports(type) method
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-05 19:25 PDT by Tsuyoshi Horo
Modified: 2022-01-13 15:19 PST (History)
14 users (show)

See Also:


Attachments
Patch (10.94 KB, patch)
2022-01-13 12:00 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (17.40 KB, patch)
2022-01-13 13:57 PST, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tsuyoshi Horo 2021-09-05 19:25:49 PDT
HTMLScriptElement.supports(type) method was introduced by the recent spec change.
https://html.spec.whatwg.org/multipage/scripting.html#dom-script-supports
https://github.com/whatwg/html/pull/7008
Comment 1 Kevin Neal 2021-09-09 10:56:46 PDT
Thank you for filing. The appropriate engineers have been copied.
Comment 2 Radar WebKit Bug Importer 2021-09-09 10:57:11 PDT
<rdar://problem/82933899>
Comment 3 Chris Dumez 2022-01-13 12:00:51 PST
Created attachment 449090 [details]
Patch
Comment 4 Geoffrey Garen 2022-01-13 13:04:56 PST
Comment on attachment 449090 [details]
Patch

r=me
Comment 5 Darin Adler 2022-01-13 13:16:44 PST
Comment on attachment 449090 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449090&action=review

> Source/WebCore/html/HTMLScriptElement.cpp:209
> +bool HTMLScriptElement::supports(const String& type)
> +{
> +    return type == "classic" || type == "module";
>  }

I would make two changes:

1) Take StringView; good practice any time we don’t need to take ownership of the String or take advantage of the hash value stored in the string

2) Put this inline in the header; why not for something this simple that could be inlined into the bindings code?
Comment 6 Chris Dumez 2022-01-13 13:57:28 PST
Created attachment 449102 [details]
Patch
Comment 7 Chris Dumez 2022-01-13 13:57:51 PST
(In reply to Darin Adler from comment #5)
> Comment on attachment 449090 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449090&action=review
> 
> > Source/WebCore/html/HTMLScriptElement.cpp:209
> > +bool HTMLScriptElement::supports(const String& type)
> > +{
> > +    return type == "classic" || type == "module";
> >  }
> 
> I would make two changes:
> 
> 1) Take StringView; good practice any time we don’t need to take ownership
> of the String or take advantage of the hash value stored in the string
> 
> 2) Put this inline in the header; why not for something this simple that
> could be inlined into the bindings code?

Done, thanks.
Comment 8 EWS 2022-01-13 15:16:50 PST
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
Comment 9 Chris Dumez 2022-01-13 15:19:52 PST
Committed r287996 (246023@trunk): <https://commits.webkit.org/246023@trunk>